From 5284fd63f1d2d9da09fd1793e49ab734557ea046 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mat=C4=9Bj=20Grabovsk=C3=BD?= <mgrabovs@redhat.com>
Date: Thu, 23 Jan 2020 13:47:50 +0100
Subject: [PATCH] koops: Avoid explicit dependency on hash size
Change the API so that koops_hash_str and koops_hash_str_ext return the
heap-allocated encoded digest instead of taking a pointer and returning
a status code.
This eliminates the need to know the size of the resulting hash in
advance, which should solve the build problems following recent
libreport changes.
---
src/include/libabrt.h | 4 +--
src/lib/kernel.c | 39 +++++++++-----------------
src/plugins/abrt-action-analyze-oops.c | 11 ++++----
tests/koops-parser.at | 14 ++++-----
4 files changed, 27 insertions(+), 41 deletions(-)
diff --git a/src/include/libabrt.h b/src/include/libabrt.h
index 33a1c5af..ab0bf6aa 100644
--- a/src/include/libabrt.h
+++ b/src/include/libabrt.h
@@ -138,9 +138,9 @@ char *kernel_tainted_short(const char *kernel_bt);
#define kernel_tainted_long abrt_kernel_tainted_long
char *kernel_tainted_long(const char *tainted_short);
#define koops_hash_str_ext abrt_koops_hash_str_ext
-int koops_hash_str_ext(char hash_str[SHA1_RESULT_LEN*2 + 1], const char *oops_buf, int frame_count, int duphas_flags);
+char *koops_hash_str_ext(const char *oops_buf, int frame_count, int duphas_flags);
#define koops_hash_str abrt_koops_hash_str
-int koops_hash_str(char hash_str[SHA1_RESULT_LEN*2 + 1], const char *oops_buf);
+char *koops_hash_str(const char *oops_buf);
#define koops_line_skip_level abrt_koops_line_skip_level
diff --git a/src/lib/kernel.c b/src/lib/kernel.c
index 7b831dac..c95bf7f9 100644
--- a/src/lib/kernel.c
+++ b/src/lib/kernel.c
@@ -586,18 +586,16 @@ void koops_extract_oopses_from_lines(GList **oops_list, const struct abrt_koops_
}
}
-int koops_hash_str_ext(char result[SHA1_RESULT_LEN*2 + 1], const char *oops_buf, int frame_count, int duphash_flags)
+char *koops_hash_str_ext(const char *oops_buf, int frame_count, int duphash_flags)
{
- char *hash_str = NULL, *error = NULL;
- int bad = 0;
+ g_autofree char *error = NULL;
+ char *digest = NULL;
struct sr_stacktrace *stacktrace = sr_stacktrace_parse(SR_REPORT_KERNELOOPS,
oops_buf, &error);
if (!stacktrace)
{
log_debug("Failed to parse koops: %s", error);
- free(error);
- bad = 1;
goto end;
}
@@ -605,43 +603,34 @@ int koops_hash_str_ext(char result[SHA1_RESULT_LEN*2 + 1], const char *oops_buf,
if (!thread)
{
log_debug("Failed to find crash thread");
- bad = 1;
goto end;
}
if (g_verbose >= 3)
{
- hash_str = sr_thread_get_duphash(thread, frame_count, NULL,
- duphash_flags|SR_DUPHASH_NOHASH);
- if (hash_str)
- log_warning("Generating duphash: '%s'", hash_str);
+ digest = sr_thread_get_duphash(thread, frame_count, NULL,
+ duphash_flags|SR_DUPHASH_NOHASH);
+ if (digest)
+ {
+ log_warning("Generating duphash: '%s'", digest);
+ free(digest);
+ }
else
log_warning("Nothing useful for duphash");
-
-
- free(hash_str);
}
- hash_str = sr_thread_get_duphash(thread, frame_count, NULL, duphash_flags);
- if (hash_str)
- {
- strncpy(result, hash_str, SHA1_RESULT_LEN*2);
- result[SHA1_RESULT_LEN*2] = '\0';
- free(hash_str);
- }
- else
- bad = 1;
+ digest = sr_thread_get_duphash(thread, frame_count, NULL, duphash_flags);
end:
sr_stacktrace_free(stacktrace);
- return bad;
+ return digest;
}
-int koops_hash_str(char result[SHA1_RESULT_LEN*2 + 1], const char *oops_buf)
+char *koops_hash_str(const char *oops_buf)
{
const int frame_count = 6;
const int duphash_flags = SR_DUPHASH_NONORMALIZE|SR_DUPHASH_KOOPS_COMPAT;
- return koops_hash_str_ext(result, oops_buf, frame_count, duphash_flags);
+ return koops_hash_str_ext(oops_buf, frame_count, duphash_flags);
}
char *koops_extract_version(const char *linepointer)
diff --git a/src/plugins/abrt-action-analyze-oops.c b/src/plugins/abrt-action-analyze-oops.c
index d7fe6886..ddd1c46d 100644
--- a/src/plugins/abrt-action-analyze-oops.c
+++ b/src/plugins/abrt-action-analyze-oops.c
@@ -60,9 +60,8 @@ int main(int argc, char **argv)
load_abrt_plugin_conf_file("oops.conf", settings);
char *oops = dd_load_text(dd, FILENAME_BACKTRACE);
- char hash_str[SHA1_RESULT_LEN*2 + 1];
- int bad = koops_hash_str(hash_str, oops);
- if (bad)
+ g_autofree char *hash_str = koops_hash_str(oops);
+ if (!hash_str)
{
error_msg("Can't find a meaningful backtrace for hashing in '%s'", dump_dir_name);
@@ -87,7 +86,7 @@ int main(int argc, char **argv)
/* We need UUID file for the local duplicates look-up and DUPHASH */
/* file is also useful because user can force ABRT to report */
/* the oops into a bug tracking system (Bugzilla). */
- bad = koops_hash_str_ext(hash_str, oops,
+ hash_str = koops_hash_str_ext(oops,
/* use no frame count limit */-1,
/* use every frame in stacktrace */0);
@@ -97,7 +96,7 @@ int main(int argc, char **argv)
free(oops);
- if (!bad)
+ if (hash_str)
{
dd_save_text(dd, FILENAME_UUID, hash_str);
dd_save_text(dd, FILENAME_DUPHASH, hash_str);
@@ -107,5 +106,5 @@ int main(int argc, char **argv)
free_map_string(settings);
- return bad;
+ return NULL != hash_str;
}
diff --git a/tests/koops-parser.at b/tests/koops-parser.at
index 4f9087ab..51a45e62 100644
--- a/tests/koops-parser.at
+++ b/tests/koops-parser.at
@@ -125,16 +125,14 @@ AT_TESTFUN([koops_hash_improve],
int run_test(const struct test_struct *test)
{
- char *oops1 = fread_full(test->filename);
- char *oops2 = fread_full(test->expected_results);
+ g_autofree char *oops1 = fread_full(test->filename);
+ g_autofree char *oops2 = fread_full(test->expected_results);
- char hash_oops1[SHA1_RESULT_LEN*2 + 1];
- koops_hash_str(hash_oops1, oops1);
- free(oops1);
+ g_autofree char *hash_oops1 = koops_hash_str(oops1);
+ g_autofree char *hash_oops2 = koops_hash_str(oops2);
- char hash_oops2[SHA1_RESULT_LEN*2 + 1];
- koops_hash_str(hash_oops2, oops2);
- free(oops2);
+ if (NULL == hash_oops1 || NULL == hash_oops2)
+ return 1;
if (!strcmp(hash_oops1, hash_oops2))
return 0;
--
2.25.0