[ABRT PATCH] abrt-cli report: accept sha1 hashes of directory names. #693

Jakub Filak jfilak at redhat.com
Tue Sep 10 09:24:24 UTC 2013


On Mon, 2013-09-09 at 17:07 +0200, Denys Vlasenko wrote:
> +char *str2hash(const char *str)
> +{
> +    static char result[SHA1_RESULT_LEN*2 + 1];
> +
> +    char hash_bytes[SHA1_RESULT_LEN];
> +    sha1_ctx_t sha1ctx;
> +    sha1_begin(&sha1ctx);
> +    sha1_hash(&sha1ctx, str, strlen(str));
> +    sha1_end(&sha1ctx, hash_bytes);
> +    bin2hex(result, hash_bytes, SHA1_RESULT_LEN)[0] = '\0';
> +    return result;
> +}

I have nothing against returning a pointer to static data but I would
return it as 'const char *' to prevent situation where one may want to
free the return value (especially in case where the function have no
documentation)

Anyway, str2hash() is the fifth implementation of same functionality in
abrt project and I would moved it to some common library (libreport
xfuncs or so) and replace all existing implementations with str2hash()
function.

Other str2hex() implementors:
  src/lib/kernel.c
  src/plugins/abrt-action-analyze-backtrace.c
  src/plugins/abrt-action-analyze-python.c
  src/plugins/abrt-action-analyze-c.c

> +
> +struct name_resolution_param {
> +    const char *shortcut;
> +    unsigned strlen_shortcut;
> +    char *found_name;
> +};

New line would be nice here.

> +static int find_dir_by_hash(struct dump_dir *dd, void *arg)
> +{
> +    struct name_resolution_param *param = arg;
> +    char *h = str2hash(dd->dd_dirname);
> +    if (strncasecmp(param->shortcut, h, param->strlen_shortcut) == 0)
> +    {
> +        if (param->found_name)
> +            error_msg_and_die(_("'%s' identifies more than one problem directory"), param->shortcut);
> +        param->found_name = xstrdup(dd->dd_dirname);
> +    }
> +    return 0;
> +}
> +
> +char *hash2dirname(const char *hash)
> +{
> +    unsigned hash_len = strlen(hash);
> +    if (!isxdigit_str(hash) || hash_len < 5)
> +        return NULL;
> +
> +    /* Try loading by dirname hash */
> +    struct name_resolution_param param = { hash, hash_len, NULL };
> +    GList *dir_list = get_problem_storages();
> +    for (GList *li = dir_list; li; li = li->next)
> +        for_each_problem_in_dir(li->data, getuid(), find_dir_by_hash, &param);
> +    return param.found_name;
> +}
> diff --git a/src/cli/abrt-cli-core.h b/src/cli/abrt-cli-core.h
> index 8eedc13..d28ef95 100644
> --- a/src/cli/abrt-cli-core.h
> +++ b/src/cli/abrt-cli-core.h
> @@ -30,4 +30,8 @@ void free_vector_of_problem_data(vector_of_problem_data_t *vector);
>  vector_of_problem_data_t *new_vector_of_problem_data(void);
>  vector_of_problem_data_t *fetch_crash_infos(GList *dir_list);
>  
> +char *str2hash(const char *str);
> +char *hash2dirname(const char *hash);
> +
> +

I'd say that these functions deserve some documentation. At least how to
process the return values because return value of str2hash() must not be
freed but return value of hash2dirname() should be freed.

>  #endif /* ABRT_CLI_CORE_H_ */



More information about the Crash-catcher mailing list