[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