assorted cleanups, memory leak fix (g_list_free in add_basics_to_problem_data)

Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>

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:<module>: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);