[ABRT PATCH] abrt-cli list: use sha1 hash as short ids instead of @N thing. rhbz#906733

[Denys Vlasenko]

Once upon a time it was meant to allow addressing problems with @N
instead of full name. However, it is problematic - what the numbering
should be if e.g. --since DATE was used? Should we scan all directories
regardless because we want to show the same @N's as in full list?
Same problem with "--full List even reported problems" versus
!--full.

Instead, use git's trick of "shortcut IDs" based on SHA hashes.
In our case, use sha1 hash of directory name.

abrt-cli info STR will attempt to use STR as sha1 hash
of directory name if STR isn't an existing directory name,
is >= 5 chars long, and is a hex string.

Example:
    $ abrt-cli list
    ...
    @d9ca58e5b3e509759c582a5c162bfe24b23150dd
    Directory:      /var/tmp/abrt/oops-2013-08-27-14:08:13-9866-0
    time:           Tue 27 Aug 2013 02:08:13 PM CEST
    $ abrt-cli info d9ca5
    Directory:      /var/tmp/abrt/oops-2013-08-27-14:08:13-9866-0
    time:           Tue 27 Aug 2013 02:08:13 PM CEST

Signed-off-by: Denys Vlasenko <dvlasenk(a)redhat.com&gt;
---
 src/cli/abrt-cli-core.c | 18 -----------
 src/cli/abrt-cli-core.h |  1 -
 src/cli/list.c          | 85 +++++++++++++++++++++++++++++++++++++++++++++++--
 3 files changed, 82 insertions(+), 22 deletions(-)

diff --git a/src/cli/abrt-cli-core.c b/src/cli/abrt-cli-core.c
index 4f2a094..26d08cc 100644
--- a/src/cli/abrt-cli-core.c
+++ b/src/cli/abrt-cli-core.c
@@ -39,24 +39,6 @@ vector_of_problem_data_t *new_vector_of_problem_data(void)
     return g_ptr_array_new_with_free_func((void (*)(void*)) &problem_data_free);
 }
 
-problem_data_t *fill_crash_info(const char *dump_dir_name)
-{
-    int sv_logmode = logmode;
-    logmode = 0; /* suppress EPERM/EACCES errors in opendir */
-    struct dump_dir *dd = dd_opendir(dump_dir_name, /*flags:*/ DD_OPEN_READONLY);
-    logmode = sv_logmode;
-
-    if (!dd)
-        return NULL;
-
-    problem_data_t *problem_data = create_problem_data_from_dump_dir(dd);
-    dd_close(dd);
-    problem_data_add(problem_data, CD_DUMPDIR, dump_dir_name,
-                            CD_FLAG_TXT + CD_FLAG_ISNOTEDITABLE + CD_FLAG_LIST);
-
-    return problem_data;
-}
-
 static int
 append_problem_data(struct dump_dir *dd, void *arg)
 {
diff --git a/src/cli/abrt-cli-core.h b/src/cli/abrt-cli-core.h
index b0f6186..8eedc13 100644
--- a/src/cli/abrt-cli-core.h
+++ b/src/cli/abrt-cli-core.h
@@ -28,7 +28,6 @@ problem_data_t *get_problem_data(vector_of_problem_data_t *vector,
unsigned i);
 
 void free_vector_of_problem_data(vector_of_problem_data_t *vector);
 vector_of_problem_data_t *new_vector_of_problem_data(void);
-problem_data_t *fill_crash_info(const char *dump_dir_name);
 vector_of_problem_data_t *fetch_crash_infos(GList *dir_list);
 
 #endif /* ABRT_CLI_CORE_H_ */
diff --git a/src/cli/list.c b/src/cli/list.c
index 06b271d..a257623 100644
--- a/src/cli/list.c
+++ b/src/cli/list.c
@@ -30,6 +30,83 @@
  *       ~/.abrt/spool and /var/tmp/abrt? needs more _meditation_.
  */
 
+static bool isxdigit_str(const char *str)
+{
+    do
+    {
+        if (*str < '0' || *str > '9')
+            if ((*str|0x20) < 'a' || (*str|0x20) > 'f')
+                return false;
+        str++;
+    } while (*str);
+    return true;
+}
+
+static 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;
+}
+
+struct name_resolution_param {
+    const char *shortcut;
+    unsigned strlen_shortcut;
+    char *found_name;
+};
+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;
+}
+
+static problem_data_t *load_problem_data(const char *dump_dir_name)
+{
+    /* First, try loading by dirname */
+    int sv_logmode = logmode;
+    logmode = 0; /* suppress EPERM/EACCES errors in opendir */
+    struct dump_dir *dd = dd_opendir(dump_dir_name, /*flags:*/ DD_OPEN_READONLY);
+    logmode = sv_logmode;
+
+    unsigned name_len = strlen(dump_dir_name);
+    /* (git requires at least 5 char hash prefix, we do the same) */
+    if (!dd && errno == ENOENT && isxdigit_str(dump_dir_name) &&
name_len >= 5)
+    {
+        /* Try loading by dirname hash */
+        struct name_resolution_param param = { dump_dir_name, name_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);
+        if (!param.found_name)
+            return NULL;
+        dd = dd_opendir(param.found_name, /*flags:*/ DD_OPEN_READONLY);
+        free(param.found_name);
+    }
+
+    if (!dd)
+        return NULL;
+
+    problem_data_t *problem_data = create_problem_data_from_dump_dir(dd);
+    problem_data_add(problem_data, CD_DUMPDIR, dd->dd_dirname,
+                            CD_FLAG_TXT + CD_FLAG_ISNOTEDITABLE + CD_FLAG_LIST);
+    dd_close(dd);
+
+    return problem_data;
+}
+
 /** Prints basic information about a crash to stdout. */
 static void print_crash(problem_data_t *problem_data, int detailed, int text_size)
 {
@@ -82,7 +159,9 @@ static void print_crash_list(vector_of_problem_data_t *crash_list, int
detailed,
                 continue;
         }
 
-        printf("@%i\n", i);
+        struct problem_item *item = g_hash_table_lookup(crash, CD_DUMPDIR);
+        if (item)
+            printf("@%s\n", str2hash(item->content));
         print_crash(crash, detailed, text_size);
         if (i != crash_list->len - 1)
             printf("\n");
@@ -158,10 +237,10 @@ int cmd_info(int argc, const char **argv)
     while (*argv)
     {
         const char *dump_dir = *argv++;
-        problem_data_t *problem = fill_crash_info(dump_dir);
+        problem_data_t *problem = load_problem_data(dump_dir);
         if (!problem)
         {
-            error_msg("no such problem directory '%s'", dump_dir);
+            error_msg(_("No such problem directory '%s'"), dump_dir);
             errs++;
             continue;
         }
-- 
1.8.1.4

[Jiri Moskovcak]

- pushed, thanks

On 09/04/2013 03:32 PM, Denys Vlasenko wrote:
>  Once upon a time it was meant to allow addressing problems with @N
>  instead of full name. However, it is problematic - what the numbering
>  should be if e.g. --since DATE was used? Should we scan all directories
>  regardless because we want to show the same @N's as in full list?
>  Same problem with "--full List even reported problems" versus
>  !--full.
>
>  Instead, use git's trick of "shortcut IDs" based on SHA hashes.
>  In our case, use sha1 hash of directory name.
>
>  abrt-cli info STR will attempt to use STR as sha1 hash
>  of directory name if STR isn't an existing directory name,
>  is >= 5 chars long, and is a hex string.
>
>  Example:
>       $ abrt-cli list
>       ...
>       @d9ca58e5b3e509759c582a5c162bfe24b23150dd
>       Directory:      /var/tmp/abrt/oops-2013-08-27-14:08:13-9866-0
>       time:           Tue 27 Aug 2013 02:08:13 PM CEST
>       $ abrt-cli info d9ca5
>       Directory:      /var/tmp/abrt/oops-2013-08-27-14:08:13-9866-0
>       time:           Tue 27 Aug 2013 02:08:13 PM CEST
>
>  Signed-off-by: Denys Vlasenko <dvlasenk(a)redhat.com&gt;
>  ---
>    src/cli/abrt-cli-core.c | 18 -----------
>    src/cli/abrt-cli-core.h |  1 -
>    src/cli/list.c          | 85 +++++++++++++++++++++++++++++++++++++++++++++++--
>    3 files changed, 82 insertions(+), 22 deletions(-)
>
>  diff --git a/src/cli/abrt-cli-core.c b/src/cli/abrt-cli-core.c
>  index 4f2a094..26d08cc 100644
>  --- a/src/cli/abrt-cli-core.c
>  +++ b/src/cli/abrt-cli-core.c
>  @@ -39,24 +39,6 @@ vector_of_problem_data_t *new_vector_of_problem_data(void)
>        return g_ptr_array_new_with_free_func((void (*)(void*)) &problem_data_free);
>    }
>
>  -problem_data_t *fill_crash_info(const char *dump_dir_name)
>  -{
>  -    int sv_logmode = logmode;
>  -    logmode = 0; /* suppress EPERM/EACCES errors in opendir */
>  -    struct dump_dir *dd = dd_opendir(dump_dir_name, /*flags:*/ DD_OPEN_READONLY);
>  -    logmode = sv_logmode;
>  -
>  -    if (!dd)
>  -        return NULL;
>  -
>  -    problem_data_t *problem_data = create_problem_data_from_dump_dir(dd);
>  -    dd_close(dd);
>  -    problem_data_add(problem_data, CD_DUMPDIR, dump_dir_name,
>  -                            CD_FLAG_TXT + CD_FLAG_ISNOTEDITABLE + CD_FLAG_LIST);
>  -
>  -    return problem_data;
>  -}
>  -
>    static int
>    append_problem_data(struct dump_dir *dd, void *arg)
>    {
>  diff --git a/src/cli/abrt-cli-core.h b/src/cli/abrt-cli-core.h
>  index b0f6186..8eedc13 100644
>  --- a/src/cli/abrt-cli-core.h
>  +++ b/src/cli/abrt-cli-core.h
>  @@ -28,7 +28,6 @@ problem_data_t *get_problem_data(vector_of_problem_data_t *vector,
> unsigned i);
>
>    void free_vector_of_problem_data(vector_of_problem_data_t *vector);
>    vector_of_problem_data_t *new_vector_of_problem_data(void);
>  -problem_data_t *fill_crash_info(const char *dump_dir_name);
>    vector_of_problem_data_t *fetch_crash_infos(GList *dir_list);
>
>    #endif /* ABRT_CLI_CORE_H_ */
>  diff --git a/src/cli/list.c b/src/cli/list.c
>  index 06b271d..a257623 100644
>  --- a/src/cli/list.c
>  +++ b/src/cli/list.c
>  @@ -30,6 +30,83 @@
>     *       ~/.abrt/spool and /var/tmp/abrt? needs more _meditation_.
>     */
>
>  +static bool isxdigit_str(const char *str)
>  +{
>  +    do
>  +    {
>  +        if (*str < '0' || *str > '9')
>  +            if ((*str|0x20) < 'a' || (*str|0x20) > 'f')
>  +                return false;
>  +        str++;
>  +    } while (*str);
>  +    return true;
>  +}
>  +
>  +static 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;
>  +}
>  +
>  +struct name_resolution_param {
>  +    const char *shortcut;
>  +    unsigned strlen_shortcut;
>  +    char *found_name;
>  +};
>  +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;
>  +}
>  +
>  +static problem_data_t *load_problem_data(const char *dump_dir_name)
>  +{
>  +    /* First, try loading by dirname */
>  +    int sv_logmode = logmode;
>  +    logmode = 0; /* suppress EPERM/EACCES errors in opendir */
>  +    struct dump_dir *dd = dd_opendir(dump_dir_name, /*flags:*/ DD_OPEN_READONLY);
>  +    logmode = sv_logmode;
>  +
>  +    unsigned name_len = strlen(dump_dir_name);
>  +    /* (git requires at least 5 char hash prefix, we do the same) */
>  +    if (!dd && errno == ENOENT && isxdigit_str(dump_dir_name) &&
> name_len >= 5)
>  +    {
>  +        /* Try loading by dirname hash */
>  +        struct name_resolution_param param = { dump_dir_name, name_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);
>  +        if (!param.found_name)
>  +            return NULL;
>  +        dd = dd_opendir(param.found_name, /*flags:*/ DD_OPEN_READONLY);
>  +        free(param.found_name);
>  +    }
>  +
>  +    if (!dd)
>  +        return NULL;
>  +
>  +    problem_data_t *problem_data = create_problem_data_from_dump_dir(dd);
>  +    problem_data_add(problem_data, CD_DUMPDIR, dd->dd_dirname,
>  +                            CD_FLAG_TXT + CD_FLAG_ISNOTEDITABLE + CD_FLAG_LIST);
>  +    dd_close(dd);
>  +
>  +    return problem_data;
>  +}
>  +
>    /** Prints basic information about a crash to stdout. */
>    static void print_crash(problem_data_t *problem_data, int detailed, int text_size)
>    {
>  @@ -82,7 +159,9 @@ static void print_crash_list(vector_of_problem_data_t *crash_list, int
> detailed,
>                    continue;
>            }
>
>  -        printf("@%i\n", i);
>  +        struct problem_item *item = g_hash_table_lookup(crash, CD_DUMPDIR);
>  +        if (item)
>  +            printf("@%s\n", str2hash(item->content));
>            print_crash(crash, detailed, text_size);
>            if (i != crash_list->len - 1)
>                printf("\n");
>  @@ -158,10 +237,10 @@ int cmd_info(int argc, const char **argv)
>        while (*argv)
>        {
>            const char *dump_dir = *argv++;
>  -        problem_data_t *problem = fill_crash_info(dump_dir);
>  +        problem_data_t *problem = load_problem_data(dump_dir);
>            if (!problem)
>            {
>  -            error_msg("no such problem directory '%s'", dump_dir);
>  +            error_msg(_("No such problem directory '%s'"), dump_dir);
>                errs++;
>                continue;
>            }
>