From 1fef96e2031952fccc4cf9ed189b50fec9360a06 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Sun, 22 May 2011 21:28:58 +0200 Subject: assorted cleanups, memory leak fix (g_list_free in add_basics_to_problem_data) Signed-off-by: Denys Vlasenko --- src/applet/applet_gtk.c | 37 ++++++++---------- src/gui-gtk/abrt-gtk.c | 2 +- src/gui-gtk/main.c | 2 +- src/include/report/report.h | 4 +- src/lib/problem_data.c | 28 +++++--------- src/lib/report.c | 60 +++++++++++++++-------------- src/plugins/abrt-action-analyze-backtrace.c | 14 +------ src/plugins/abrt-action-analyze-c.c | 15 +------- src/plugins/abrt-action-analyze-oops.c | 14 +------ src/plugins/abrt-action-analyze-python.c | 14 +------ 10 files changed, 70 insertions(+), 120 deletions(-) diff --git a/src/applet/applet_gtk.c b/src/applet/applet_gtk.c index 59c21ef8..74347963 100644 --- a/src/applet/applet_gtk.c +++ b/src/applet/applet_gtk.c @@ -23,24 +23,24 @@ static gboolean persistent_notification; #if !defined(NOTIFY_VERSION_MINOR) || (NOTIFY_VERSION_MAJOR == 0 && NOTIFY_VERSION_MINOR >= 6) -static gboolean server_has_persistence (void) +static gboolean server_has_persistence(void) { gboolean has; - GList *caps; - GList *l; + GList *caps; + GList *l; - caps = notify_get_server_caps (); - if (caps == NULL) { - fprintf (stderr, "Failed to receive server caps.\n"); - return FALSE; + caps = notify_get_server_caps(); + if (caps == NULL) + { + error_msg("Failed to receive server caps"); + return FALSE; } - l = g_list_find_custom (caps, "persistence", (GCompareFunc)strcmp); - has = l != NULL; + l = g_list_find_custom(caps, "persistence", (GCompareFunc)strcmp); + has = (l != NULL); - g_list_foreach (caps, (GFunc) g_free, NULL); - g_list_free (caps); - VERB1 log("notify server %s support pesistence\n", has ? "DOES" : "DOESN'T"); + list_free_with_free(caps); + VERB1 log("notify server %s support pesistence", has ? "DOES" : "DOESN'T"); return has; } #endif @@ -86,7 +86,7 @@ static void action_report(NotifyNotification *notification, gchar *action, gpoin struct applet *applet = (struct applet *)user_data; if (applet->ap_daemon_running) { - analyze_and_report_dir(applet->ap_last_crash_id, NOWAIT); + analyze_and_report_dir(applet->ap_last_crash_id, LIBREPORT_NOWAIT); GError *err = NULL; notify_notification_close(notification, &err); if (err != NULL) @@ -361,16 +361,13 @@ void set_icon_tooltip(struct applet *applet, const char *format, ...) return; va_list args; - int n; char *buf; - // xvasprintf? va_start(args, format); - buf = NULL; - n = vasprintf(&buf, format, args); + buf = xvasprintf(format, args); va_end(args); - gtk_status_icon_set_tooltip_text(applet->ap_status_icon, (n >= 0 && buf) ? buf : ""); + gtk_status_icon_set_tooltip_text(applet->ap_status_icon, buf); free(buf); } @@ -392,6 +389,7 @@ void show_crash_notification(struct applet *applet, const char* crash_dir, const notify_notification_update(notification, _("A Problem has Occurred"), buf, NULL); free(buf); + GError *err = NULL; notify_notification_show(notification, &err); if (err != NULL) @@ -412,7 +410,7 @@ void show_msg_notification(struct applet *applet, const char *format, ...) /* we don't want to show any buttons now, maybe later we can add action binded to message like >>Clear old dumps<< for quota exceeded - */ + */ NotifyNotification *notification = new_warn_notification(); notify_notification_add_action(notification, "OPEN_MAIN_WINDOW", _("Open ABRT"), NOTIFY_ACTION_CALLBACK(action_open_gui), @@ -447,4 +445,3 @@ void hide_icon(struct applet *applet) gtk_status_icon_set_visible(applet->ap_status_icon, false); stop_animate_icon(applet); } - diff --git a/src/gui-gtk/abrt-gtk.c b/src/gui-gtk/abrt-gtk.c index fe061829..f6e9c5c2 100644 --- a/src/gui-gtk/abrt-gtk.c +++ b/src/gui-gtk/abrt-gtk.c @@ -109,7 +109,7 @@ static void on_row_activated_cb(GtkTreeView *treeview, GtkTreePath *path, GtkTre gtk_tree_model_get_value(store, &iter, COLUMN_DUMP_DIR, &d_dir); const char *dirname= g_value_get_string(&d_dir); - analyze_and_report_dir(dirname, NOWAIT); + analyze_and_report_dir(dirname, LIBREPORT_NOWAIT); } } } diff --git a/src/gui-gtk/main.c b/src/gui-gtk/main.c index bb57ce7d..f76f62f3 100644 --- a/src/gui-gtk/main.c +++ b/src/gui-gtk/main.c @@ -56,7 +56,7 @@ static void handle_signal(int signo) uint8_t sig_caught = signo; if (write(s_signal_pipe[1], &sig_caught, 1)) - /* we ignore result, if() shuts up stupid compiler */; + /* we ignore result, if () shuts up stupid compiler */; errno = save_errno; } diff --git a/src/include/report/report.h b/src/include/report/report.h index f57d5277..e441875b 100644 --- a/src/include/report/report.h +++ b/src/include/report/report.h @@ -22,8 +22,8 @@ #include "problem_data.h" enum { - NOWAIT = 0, - WAIT = (1 << 0), /* wait for report to finish and reload the problem data */ + LIBREPORT_NOWAIT = 0, + LIBREPORT_WAIT = (1 << 0), /* wait for report to finish and reload the problem data */ }; diff --git a/src/lib/problem_data.c b/src/lib/problem_data.c index 22c06ff2..87821afd 100644 --- a/src/lib/problem_data.c +++ b/src/lib/problem_data.c @@ -70,10 +70,9 @@ void add_basics_to_problem_data(problem_data_t *pd) if (get_problem_item_content_or_NULL(pd, FILENAME_DUPHASH) == NULL) { /* start hash */ - static sha1_ctx_t sha1ctx; - unsigned char hash_bytes[SHA1_RESULT_LEN]; - char hash_str[SHA1_RESULT_LEN*2 + 1]; + sha1_ctx_t sha1ctx; sha1_begin(&sha1ctx); + /* * To avoid spurious hash differences, sort keys so that elements are * always processed in the same order: @@ -94,20 +93,13 @@ void add_basics_to_problem_data(problem_data_t *pd) continue; sha1_hash(&sha1ctx, item->content, strlen(item->content)); } + g_list_free(list); + /* end hash */ + char hash_bytes[SHA1_RESULT_LEN]; sha1_end(&sha1ctx, hash_bytes); - - unsigned len = SHA1_RESULT_LEN; - unsigned char *s = hash_bytes; - char *d = hash_str; - while (len) - { - *d++ = "0123456789abcdef"[*s >> 4]; - *d++ = "0123456789abcdef"[*s & 0xf]; - s++; - len--; - } - *d = '\0'; + char hash_str[SHA1_RESULT_LEN*2 + 1]; + bin2hex(hash_str, hash_bytes, SHA1_RESULT_LEN)[0] = '\0'; add_to_problem_data(pd, FILENAME_DUPHASH, hash_str); } @@ -121,7 +113,7 @@ void add_basics_to_problem_data(problem_data_t *pd) if (read > 0) { buf[read] = 0; - VERB2 log("reporting initiated from: %s\n", buf); + VERB2 log("reporting initiated from: %s", buf); add_to_problem_data(pd, FILENAME_EXECUTABLE, buf); } free(exe); @@ -130,11 +122,11 @@ void add_basics_to_problem_data(problem_data_t *pd) /* FIXME: component should be taken from rpm using librpm * which means we need to link against it :( * or run rpm -qf executable ?? - */ + */ /* Fedora/RHEL rpm specific piece of code */ const char *component = get_problem_item_content_or_NULL(pd, FILENAME_COMPONENT); //FIXME: this REALLY needs to go away, or every report will be assigned to abrt - if(component == NULL) // application didn't specify component + if (component == NULL) // application didn't specify component add_to_problem_data(pd, FILENAME_COMPONENT, "abrt"); //#endif } diff --git a/src/lib/report.c b/src/lib/report.c index d6b45fcf..99a3f919 100644 --- a/src/lib/report.c +++ b/src/lib/report.c @@ -30,7 +30,13 @@ static int run_reporter_ui(char **args, int flags) */ pid_t pid = vfork(); - if (pid == 0) + if (pid < 0) /* error */ + { + perror_msg("vfork"); + return -1; + } + + if (pid == 0) /* child */ { /* Some callers set SIGCHLD to SIG_IGN. * However, reporting spawns child processes. @@ -47,31 +53,29 @@ static int run_reporter_ui(char **args, int flags) */ path = "bug-reporting-wizard"; execvp(path, args); - perror_msg_and_die("Can't execute %s", "bug-reporting-wizard"); + perror_msg_and_die("Can't execute %s", path); } - else if(pid > 0) + + /* parent */ + if (flags & LIBREPORT_WAIT) { - if (flags & WAIT) + int status; + pid_t p = waitpid(pid, &status, 0); + if (p <= 0) { - int status; - pid_t p = waitpid(pid, &status, 0); - if(p <= 0) - { - perror_msg("can't waitpid"); - return EXIT_FAILURE; - } - if (WIFEXITED(status)) - { - VERB2 log("reporting finished with exitcode: status=%d\n", WEXITSTATUS(status)); - return WEXITSTATUS(status); - } - else // (WIFSIGNALED(status)) - { - VERB2 log("reporting killed by signal %d\n", WTERMSIG(status)); - return WTERMSIG(status) + 128; - } + perror_msg("can't waitpid"); + return -1; } + if (WIFEXITED(status)) + { + VERB2 log("reporting finished with exitcode %d", WEXITSTATUS(status)); + return WEXITSTATUS(status); + } + /* child died from a signal: WIFSIGNALED(status) should be true */ + VERB2 log("reporting killed by signal %d", WTERMSIG(status)); + return WTERMSIG(status) + 128; } + return 0; } @@ -84,8 +88,7 @@ int analyze_and_report_dir(const char* dirname, int flags) args[2] = (char *)dirname; args[3] = NULL; - run_reporter_ui(args, flags); - return 0; + return run_reporter_ui(args, flags); } /* analyzes AND reports a problem saved on disk @@ -99,14 +102,14 @@ int analyze_and_report(problem_data_t *pd, int flags) return -1; char *dir_name = xstrdup(dd->dd_dirname); dd_close(dd); - VERB2 log("Temp problem dir: '%s'\n", dir_name); + VERB2 log("Temp problem dir: '%s'", dir_name); result = analyze_and_report_dir(dir_name, flags); /* if we wait for reporter to finish, we can clean the tmp dir * and we should reload the problem data, so caller doesn't see the stalled * data */ - if (flags & WAIT) + if (flags & LIBREPORT_WAIT) { g_hash_table_remove_all(pd); dd = dd_opendir(dir_name, 0); @@ -138,9 +141,8 @@ int report_dir(const char* dirname) args[3] = (char *)dirname; args[4] = NULL; - int flags = WAIT; - int status; - status = run_reporter_ui(args, flags); + int flags = LIBREPORT_WAIT; + int status = run_reporter_ui(args, flags); return status; } @@ -158,7 +160,7 @@ int report(problem_data_t *pd) dd_create_basic_files(dd, getuid()); char *dir_name = xstrdup(dd->dd_dirname); dd_close(dd); - VERB2 log("Temp problem dir: '%s'\n", dir_name); + VERB2 log("Temp problem dir: '%s'", dir_name); int result = report_dir(dir_name); /* here we always wait for reporter to finish, we can clean the tmp dir diff --git a/src/plugins/abrt-action-analyze-backtrace.c b/src/plugins/abrt-action-analyze-backtrace.c index 5d8c77f6..43293c0b 100644 --- a/src/plugins/abrt-action-analyze-backtrace.c +++ b/src/plugins/abrt-action-analyze-backtrace.c @@ -26,23 +26,13 @@ static const char *dump_dir_name = "."; static void create_hash(char hash_str[SHA1_RESULT_LEN*2 + 1], const char *pInput) { - unsigned char hash_bytes[SHA1_RESULT_LEN]; + char hash_bytes[SHA1_RESULT_LEN]; sha1_ctx_t sha1ctx; sha1_begin(&sha1ctx); sha1_hash(&sha1ctx, pInput, strlen(pInput)); sha1_end(&sha1ctx, hash_bytes); - unsigned len = SHA1_RESULT_LEN; - unsigned char *s = hash_bytes; - char *d = hash_str; - while (len) - { - *d++ = "0123456789abcdef"[*s >> 4]; - *d++ = "0123456789abcdef"[*s & 0xf]; - s++; - len--; - } - *d = '\0'; + bin2hex(hash_str, hash_bytes, SHA1_RESULT_LEN)[0] = '\0'; //log("hash:%s str:'%s'", hash_str, pInput); } diff --git a/src/plugins/abrt-action-analyze-c.c b/src/plugins/abrt-action-analyze-c.c index 487fb8f7..240ca2a2 100644 --- a/src/plugins/abrt-action-analyze-c.c +++ b/src/plugins/abrt-action-analyze-c.c @@ -21,24 +21,13 @@ static void create_hash(char hash_str[SHA1_RESULT_LEN*2 + 1], const char *pInput) { - unsigned char hash_bytes[SHA1_RESULT_LEN]; - + char hash_bytes[SHA1_RESULT_LEN]; sha1_ctx_t sha1ctx; sha1_begin(&sha1ctx); sha1_hash(&sha1ctx, pInput, strlen(pInput)); sha1_end(&sha1ctx, hash_bytes); - unsigned len = SHA1_RESULT_LEN; - unsigned char *s = hash_bytes; - char *d = hash_str; - while (len) - { - *d++ = "0123456789abcdef"[*s >> 4]; - *d++ = "0123456789abcdef"[*s & 0xf]; - s++; - len--; - } - *d = '\0'; + bin2hex(hash_str, hash_bytes, SHA1_RESULT_LEN)[0] = '\0'; //log("hash:%s str:'%s'", hash_str, pInput); } diff --git a/src/plugins/abrt-action-analyze-oops.c b/src/plugins/abrt-action-analyze-oops.c index 9485d3c8..02ba3e96 100644 --- a/src/plugins/abrt-action-analyze-oops.c +++ b/src/plugins/abrt-action-analyze-oops.c @@ -114,23 +114,13 @@ static void hash_oops_str(char hash_str[SHA1_RESULT_LEN*2 + 1], char *oops_buf, gen_hash: ; - unsigned char hash_bytes[SHA1_RESULT_LEN]; + char hash_bytes[SHA1_RESULT_LEN]; sha1_ctx_t sha1ctx; sha1_begin(&sha1ctx); sha1_hash(&sha1ctx, oops_buf, dst - oops_buf); sha1_end(&sha1ctx, hash_bytes); - unsigned len = SHA1_RESULT_LEN; - unsigned char *s = hash_bytes; - char *d = hash_str; - while (len) - { - *d++ = "0123456789abcdef"[*s >> 4]; - *d++ = "0123456789abcdef"[*s & 0xf]; - s++; - len--; - } - *d = '\0'; + bin2hex(hash_str, hash_bytes, SHA1_RESULT_LEN)[0] = '\0'; } int main(int argc, char **argv) diff --git a/src/plugins/abrt-action-analyze-python.c b/src/plugins/abrt-action-analyze-python.c index 5a33fb8d..c991cfc2 100644 --- a/src/plugins/abrt-action-analyze-python.c +++ b/src/plugins/abrt-action-analyze-python.c @@ -53,7 +53,7 @@ int main(int argc, char **argv) /* Hash 1st line of backtrace and save it as UUID and DUPHASH */ /* "example.py:1::ZeroDivisionError: integer division or modulo by zero" */ - unsigned char hash_bytes[SHA1_RESULT_LEN]; + char hash_bytes[SHA1_RESULT_LEN]; sha1_ctx_t sha1ctx; sha1_begin(&sha1ctx); const char *bt_end = strchrnul(bt, '\n'); @@ -62,17 +62,7 @@ int main(int argc, char **argv) free(bt); char hash_str[SHA1_RESULT_LEN*2 + 1]; - unsigned len = SHA1_RESULT_LEN; - unsigned char *s = hash_bytes; - char *d = hash_str; - while (len) - { - *d++ = "0123456789abcdef"[*s >> 4]; - *d++ = "0123456789abcdef"[*s & 0xf]; - s++; - len--; - } - *d = '\0'; + bin2hex(hash_str, hash_bytes, SHA1_RESULT_LEN)[0] = '\0'; dd_save_text(dd, FILENAME_UUID, hash_str); dd_save_text(dd, FILENAME_DUPHASH, hash_str); -- cgit