Discussion:
[ABRT PATCH] abrt-cli list: use sha1 hash as short ids instead of @N thing. rhbz#906733
Denys Vlasenko
2013-09-04 13:32:01 UTC
Permalink
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-H+wXaHxf7aLQT0dZR+***@public.gmane.org>
---
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
2013-09-05 10:08:07 UTC
Permalink
- pushed, thanks
Post by Denys Vlasenko
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
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.
$ 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
---
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;
}
+ struct problem_item *item = g_hash_table_lookup(crash, CD_DUMPDIR);
+ if (item)
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;
}
Loading...