[ABRT PATCH 1/2] problem api: publish problem conditions
Denys Vlasenko
dvlasenk at redhat.com
Mon May 20 15:19:35 UTC 2013
On 05/20/2013 03:56 PM, Jakub Filak wrote:
> Related to #650
>
> Signed-off-by: Jakub Filak <jfilak at redhat.com>
> ---
> src/include/problem_api.h | 140 +++++++++++++++++++++++++++++++++++++---
> src/lib/problem_api.c | 159 ++++++++++++++++++++++++++++++++++++----------
> 2 files changed, 256 insertions(+), 43 deletions(-)
>
> diff --git a/src/include/problem_api.h b/src/include/problem_api.h
> index 3b1dfd3..e1b9c0c 100644
> --- a/src/include/problem_api.h
> +++ b/src/include/problem_api.h
> @@ -23,22 +23,142 @@
> /*
> * Structure for simple conditions based on problem fields
> */
> -struct problem_condition
> -{
> - /* a name of filed required by evaluate function */
> - const char *field_name;
> - /* extra data passed to evaluate function */
> - const void *args;
> - /* evaluate function returning TRUE if condition was passed */
> - bool (*evaluate)(const char *, const void *);
> -};
> +struct problem_condition;
> +
> +/*
> + * Function pointer for evaluation function
> + *
> + * @param field_value Checked value
> + * @param args User arguments
> + */
> +typedef bool (* problem_condition_evaluate)(const char *field_value, void *args);
> +
> +/*
> + * Function pointer for problem arguments destroy function
> + *
> + * @param args User arguments
> + */
> +typedef void (* problem_condition_destroy_args)(void *args);
> +
> +/*
> + * Allocates and initialize a new problem condition
either
"Allocate and initialize"
or
"Allocates and initializes"
> + *
> + * @param field_name Name of tested element
> + * @param args Arguments passed to @evaluate function
> + * @param eval Condition evaluation function
> + * @param destroy Function to free the memory allocated for the args
> + */
> +struct problem_condition *
> +problem_condition_new(const char *field_name, void *args,
> + problem_condition_evaluate eval, problem_condition_destroy_args destroy);
What if you need to, say, compare *two* fields?
You will extend it again?
> +/*
> + * Evaluates a NULL-terminated list of problem conditions as a logical conjunction
> + *
> + * @param dd Data for evaluation
> + * @param condition NULL terminated list of condition
> + * @return True if @condition is satisfied by @dd
> + */
> +bool
> +problem_condition_evaluate_dump_dir_and(struct dump_dir *dd,
> + const struct problem_condition *const *condition);
> +
> +/*
> + * Evaluates a NULL-terminated list of problem conditions as a logical conjunction
> + *
> + * @param pd Data for evaluation
> + * @param condition NULL terminated list of condition
> + * @return True if @condition is satisfied by @pd
> + */
> +bool
> +problem_condition_evaluate_problem_data_and(problem_data_t *pd,
> + const struct problem_condition *const *condition);
> +/*
> + * Frees the memory allocate for the condition
> + *
> + * @param cond Freed memory
> + */
> +void
> +problem_condition_free(struct problem_condition *cond);
> +
> +/*
> + * Allocates and initialize a new problem condition satisfied if value of @file_name
> + * is in range <@from, @to>
> + *
> + * @param file_name Name of tested element
> + * @param from First accepted timestamp of last occurrence of a problem
> + * @param to Last accepted timestamp of last occurrence of a problem. If 0 then current timestamp is being used.
> + * @return Mallocate memory which must be free by @time_interval_problem_condition_free()
The patches do not have time_interval_problem_condition_free() function.
> + */
> +struct problem_condition *
> +time_interval_problem_condition_new(const char *file_name, unsigned long from, unsigned long to);
> +
> +/*
> + * Allocates and initialize a new problem condition satisfied if @file_name's
> + * value is empty
> + *
> + * @param file_name Name of tested element
> + * @param inverted Inverts the result (if not empty)
> + * @return Mallocate memory which must be free by @time_interval_problem_condition_free()
> + */
> +struct problem_condition *
> +empty_element_problem_condition_new(const char *file_name, bool inverted);
>
> /* Retrieves the list of directories currently used as a problem storage
> * The result must be freed by caller
> * @returns List of strings representing the full path to dirs
> */
> GList *get_problem_storages();
> +
> +/*
> + * Function pointer for callback in @foreach_problem_dir_in()
> + *
> + * @param dd Opened dump directory
> + * @param user_args User arguments
> + */
> +typedef void (* dump_dir_callback)(struct dump_dir *dd, void *user_args);
> +
> +/*
> + * Goes through all problems and passes an opened dump directory to @callback
> + * function for problems accessible by caller_uid and problems for which
> + * @condition gets TRUE
> + *
> + * @param path Base dump directories location
> + * @param uid To filter out not accessible problems; -1 to disable this check
> + * @param condition a NULL-terminated list of problem conditions evaluated
> + * as conjunction, can be NULL (means always TRUE)
> + * @param callback Called for each accessible dump directory which satisfies @condition
> + * @param user_data A user data for @callback
> + */
> +void foreach_problem_dir_matching(const char *path,
> + uid_t uid,
> + const struct problem_condition *const *condition,
> + dump_dir_callback callback,
> + void *user_data);
> +
> +/*
> + * Goes through all problems and selects only problems accessible by caller_uid and
> + * problems for which @condition gets TRUE
> + *
> + * @param condition a NULL-terminated list of problem conditions evaluated
> + * as conjunction, can be NULL (means always TRUE)
> + */
> +GList *get_problem_dirs_for_cond(const char *path,
> + uid_t uid,
> + const struct problem_condition *const *condition);
> +
> GList *get_problem_dirs_for_uid(uid_t uid, const char *dump_location);
> +
> +/*
> + * Finds problems which were created in the interval and @element holds @value.
> + *
> + * @param uid To filter out not accessible problems; -1 to disable this check
> + * @param element File inside of dump directory. NULL to disable this check
> + * @param value Required value stored in @element
> + * @param timestamp_from First accepted timestamp of last occurrence of a problem
> + * @param timestamp_to Last accepted timestamp of last occurrence of a problem. If 0 then current timestamp is being used.
> + * @param path Base directory
> + */
> GList *get_problem_dirs_for_element_in_time(uid_t uid,
> const char *element,
> const char *value,
> @@ -52,4 +172,4 @@ GList *get_problem_dirs_for_element_in_time(uid_t uid,
> * @since[in]
> * @returns count of problems
> */
> -unsigned int get_problems_count(GList *paths, unsigned long since);
> \ No newline at end of file
> +unsigned int get_problems_count(GList *paths, unsigned long since);
> diff --git a/src/lib/problem_api.c b/src/lib/problem_api.c
> index d940767..8cf6639 100644
> --- a/src/lib/problem_api.c
> +++ b/src/lib/problem_api.c
> @@ -22,18 +22,63 @@
> #include "problem_api.h"
>
> /*
> + * Structure for simple conditions based on problem fields
> + */
> +struct problem_condition
> +{
> + /* a name of filed required by evaluate function */
field
> + char *field_name;
> + /* extra data passed to evaluate function */
> + void *args;
> + /* evaluate function returning TRUE if condition was passed */
condition was satisfied
> + problem_condition_evaluate evaluate;
> + /* args destroy function */
> + problem_condition_destroy_args destroy;
> +};
> +
> +struct problem_condition *
> +problem_condition_new(const char *field_name, void *args,
> + problem_condition_evaluate evaluate,
> + problem_condition_destroy_args destroy)
> +{
> + struct problem_condition *cond = xmalloc(sizeof(*cond));
> + cond->field_name = xstrdup(field_name);
> + cond->args = args;
> + cond->evaluate = evaluate;
> + cond->destroy = destroy;
> + return cond;
> +}
> +
> +void
> +problem_condition_free(struct problem_condition *cond)
> +{
> + if (cond == NULL)
> + return;
> +
> + free(cond->field_name);
> +
> + if (cond->destroy != NULL)
> + cond->destroy(cond->args);
> +
> + free(cond);
> +}
> +
> +/*
> * Evaluates a NULL-terminated list of problem conditions as a logical conjunction
> */
> -static bool problem_condition_evaluate_and(struct dump_dir *dd,
> +static bool problem_condition_evaluate_and(void *data,
> + char *(* getter)(void *, const char *),
> + bool needfree,
> const struct problem_condition *const *condition)
> {
> /* We stop on the first FALSE condition */
> while (condition && *condition != NULL)
> {
> const struct problem_condition *c = *condition;
> - char *field_data = dd_load_text(dd, c->field_name);
> + char *field_data = getter(data, c->field_name);
> bool value = c->evaluate(field_data, c->args);
> - free(field_data);
> + if (needfree)
> + free(field_data);
> if (!value)
> return false;
> ++condition;
> @@ -42,26 +87,27 @@ static bool problem_condition_evaluate_and(struct dump_dir *dd,
> return true;
> }
>
> -/*
> - * Goes through all problems and selects only problems accessible by caller_uid and
> - * problems for which an and_filter gets TRUE
> - *
> - * @param condition a NULL-terminated list of problem conditions evaluated
> - * as conjunction, can be NULL (means always TRUE)
> - */
> -static GList* scan_directory(const char *path,
> - uid_t caller_uid,
> - const struct problem_condition *const *condition)
> +bool problem_condition_evaluate_dump_dir_and(struct dump_dir *dd,
> + const struct problem_condition *const *condition)
> {
> - GList *list = NULL;
> + return problem_condition_evaluate_and(dd, (char *(*)(void *, const char*))dd_load_text, /*free*/true, condition);
> +}
> +
> +bool problem_condition_evaluate_problem_data_and(problem_data_t *pd,
> + const struct problem_condition *const *condition)
> +{
> + return problem_condition_evaluate_and(pd, (char *(*)(void *, const char*))problem_data_get_content_or_NULL, /*free*/false, condition);
> +}
>
> +void foreach_problem_dir_matching(const char *path,
> + uid_t caller_uid,
> + const struct problem_condition *const *condition,
> + dump_dir_callback callback,
> + void *user_data)
> +{
> DIR *dp = opendir(path);
> if (!dp)
> - {
> - /* We don't want to yell if, say, $XDG_CACHE_DIR/abrt/spool doesn't exist */
> - //perror_msg("Can't open directory '%s'", path);
> - return list;
> - }
> + return;
>
> struct dirent *dent;
> while ((dent = readdir(dp)) != NULL)
> @@ -70,7 +116,7 @@ static GList* scan_directory(const char *path,
> continue; /* skip "." and ".." */
>
> char *full_name = concat_path_file(path, dent->d_name);
> - if (dump_dir_accessible_by_uid(full_name, caller_uid))
> + if (caller_uid == -1 || dump_dir_accessible_by_uid(full_name, caller_uid))
> {
> /* Silently ignore *any* errors, not only EACCES.
> * We saw "lock file is locked by process PID" error
> @@ -85,9 +131,9 @@ static GList* scan_directory(const char *path,
> */
> if (dd)
> {
> - if (!condition || problem_condition_evaluate_and(dd, condition))
> + if (!condition || problem_condition_evaluate_dump_dir_and(dd, condition))
> {
> - list = g_list_prepend(list, full_name);
> + callback(dd, user_data);
> full_name = NULL;
> }
> dd_close(dd); //doesn't fail even if dd == NULL
> @@ -96,6 +142,20 @@ static GList* scan_directory(const char *path,
> free(full_name);
> }
> closedir(dp);
> +}
> +
> +static void
> +prepend_problem_dir_name(struct dump_dir *dd, GList **list)
> +{
> + *list = g_list_prepend(*list, xstrdup(dd->dd_dirname));
> +}
> +
> +GList* get_matching_problem_dirs(const char *path,
> + uid_t caller_uid,
> + const struct problem_condition *const *condition)
> +{
> + GList *list = NULL;
> + foreach_problem_dir_matching(path, caller_uid, condition, (dump_dir_callback)prepend_problem_dir_name, &list);
>
> /* Why reverse?
> * Because N*prepend+reverse is faster than N*append
> @@ -114,18 +174,50 @@ struct time_interval
> * A problem condition evaluate function for checking of the TIME field against
> * an allowed interval
> *
> - * @param field_data a content from the PID field
> + * @param field_data a content of a field
> * @param args a pointer to an instance of struct time_interval
> * @return TRUE if a field value is in a specified interval; otherwise FALSE
> */
> -static bool time_interval_problem_condition(const char *field_data, const void *args)
> +static bool time_interval_problem_condition(const char *field_data, void *args)
> {
> const struct time_interval *const interval = (const struct time_interval *)args;
> - const time_t timestamp = atol(field_data);
> + const time_t timestamp = field_data ? atol(field_data) : 0;
>
> return interval->from <= timestamp && timestamp <= interval->to;
> }
>
> +struct problem_condition *
> +time_interval_problem_condition_new(const char *field_name, unsigned long from, unsigned long to)
> +{
> + struct time_interval *ti = xmalloc(sizeof(*ti));
> + ti->from = from;
> + if (to == 0)
> + {
> + /* use the current time if timestamp_to is 0 */
> + struct timeval tv;
> + gettimeofday(&tv, NULL);
> + ti->to = tv.tv_sec;
> + }
> + else
> + ti->to = to;
> +
> + return problem_condition_new(field_name, ti, time_interval_problem_condition, free);
> +}
> +
> +static bool empty_element_problem_condition(const char *field_data, void *args)
> +{
> + if ((bool)args)
> + return field_data != NULL;
> +
> + return field_data == NULL;
> +}
> +
> +struct problem_condition *
> +empty_element_problem_condition_new(const char *file_name, bool inverted)
> +{
> + return problem_condition_new(file_name, (void *)inverted, empty_element_problem_condition, NULL);
> +}
> +
> /*
> * A problem condition evaluate function passed if strings are equal
> *
> @@ -133,14 +225,15 @@ static bool time_interval_problem_condition(const char *field_data, const void *
> * @param args a checked string
> * @return TRUE if both strings are equal; otherwise FALSE
> */
> -static bool equal_string_problem_condition(const char *field_data, const void *args)
> +static bool equal_string_problem_condition(const char *field_data, void *args)
> {
> return !strcmp(field_data, (const char *)args);
> }
>
> GList *get_problem_dirs_for_uid(uid_t uid, const char *dump_location)
> {
> - GList *dirs = scan_directory(dump_location, uid, NULL);
> + /* NULL condition means get all valid and accessible dump dirs */
> + GList *dirs = get_matching_problem_dirs(dump_location, uid, /*condition*/NULL);
> return dirs;
> }
>
> @@ -163,8 +256,8 @@ GList *get_problem_dirs_for_element_in_time(uid_t uid,
>
>
> const struct problem_condition elementc = {
> - .field_name = element,
> - .args = value,
> + .field_name = (char *)element,
> + .args = (void *)value,
> .evaluate = equal_string_problem_condition,
> };
>
> @@ -174,8 +267,8 @@ GList *get_problem_dirs_for_element_in_time(uid_t uid,
> };
>
> const struct problem_condition timec = {
> - .field_name = FILENAME_LAST_OCCURRENCE,
> - .args = &interval,
> + .field_name = (char *)FILENAME_LAST_OCCURRENCE,
> + .args = (void *)&interval,
> .evaluate = time_interval_problem_condition
> };
>
> @@ -184,7 +277,7 @@ GList *get_problem_dirs_for_element_in_time(uid_t uid,
> element ? &elementc : NULL,
> NULL
> };
> - GList *dirs = scan_directory(dump_location, uid, condition);
> + GList *dirs = get_matching_problem_dirs(dump_location, uid, condition);
> return dirs;
> }
>
> @@ -237,4 +330,4 @@ unsigned int get_problems_count(GList *paths, unsigned long since)
> list_free_with_free(pths);
>
> return pci.problem_count;
> -}
> \ No newline at end of file
> +}
I don't think coding these things in mind-bending callback form makes sense.
Why don't you just open-code it in a human-readable form?
More information about the Crash-catcher
mailing list