v0.9.6 bug fix release
authorTim Pepper <timothy.c.pepper@linux.intel.com>
Thu, 4 Oct 2012 21:45:48 +0000 (14:45 -0700)
committerTim Pepper <timothy.c.pepper@linux.intel.com>
Thu, 4 Oct 2012 21:45:48 +0000 (14:45 -0700)
A new release including small fixes:
- remove linkage to libproxy (curl knows about proxies already)
- clean up memory management of strings in summary report, simplify and
  insure each report field's string is populated or set once to a default
  "Unknown" placeholder string
- removed no longer needed check for a "wrapped" app/core
- fix valgrind complain about loop which strips 0x1a's
- fix double "#0" line in backtrace
- additional error handling / requeue for failed report submissions

Signed-off-by: Tim Pepper <timothy.c.pepper@linux.intel.com>
configure.ac
src/Makefile.am
src/coredump.c
src/submit.c

index 96c1700..fbcbde1 100644 (file)
@@ -1,5 +1,5 @@
 AC_PREREQ([2.68])
-AC_INIT([nitra-corewatcher],[0.9.5],[timothy.c.pepper@linux.intel.com])
+AC_INIT([nitra-corewatcher],[0.9.6],[timothy.c.pepper@linux.intel.com])
 AM_INIT_AUTOMAKE([foreign -Wall -Werror])
 AC_CONFIG_FILES([Makefile src/Makefile])
 AC_CONFIG_SRCDIR([src/corewatcher.c])
@@ -11,10 +11,8 @@ AM_PROG_CC_C_O
 AC_PROG_INSTALL
 
 # PkgConfig tests
-PKG_CHECK_MODULES([GLIB2], [glib-2.0])
-PKG_CHECK_MODULES([LIBPROXY], [libproxy-1.0])
-PKG_CHECK_MODULES([LIBNOTIFY], [libnotify])
-PKG_CHECK_MODULES([LIBCURL], [libcurl])
+PKG_CHECK_MODULES([glib], [glib-2.0 gthread-2.0])
+PKG_CHECK_MODULES([curl], [libcurl])
 
 # Checks for header files.
 AC_CHECK_HEADERS([stdio.h assert.h sys/types.h sys/stat.h dirent.h signal.h errno.h sched.h fcntl.h stdlib.h string.h sys/time.h syslog.h unistd.h asm/unistd.h])
index 3a449e4..af9b2c2 100644 (file)
@@ -15,17 +15,8 @@ corewatcher_SOURCES = \
        find_file.c \
        submit.c
 
-corewatcher_CFLAGS = \
-       $(AM_CFLAGS) \
-       $(LIBPROXY_CFLAGS) \
-       $(LIBNOTIFY_CFLAGS) \
-       $(LIBCURL_CFLAGS)
-
-corewatcher_LDADD = \
-       $(GLIB2_LIBS) \
-       $(LIBPROXY_LIBS) \
-       $(LIBNOTIFY_LIBS) \
-       $(LIBCURL_LIBS)
-
 noinst_HEADERS = \
        corewatcher.h
+
+AM_CPPFLAGS = $(AM_CFLAGS) $(glib_CFLAGS) ${curl_CFLAGS}
+corewatcher_LDADD = $(glib_LIBS) ${curl_LIBS}
index 2990cc7..3bc7540 100644 (file)
@@ -70,10 +70,8 @@ static char *get_release(void)
        size_t dummy = 0;
 
        file = fopen("/etc/os-release", "r");
-       if (!file) {
-               line = strdup("Unknown");
-               return line;
-       }
+       if (!file)
+               return NULL;
 
        while (!feof(file)) {
                if (getline(&line, &dummy, file) == -1)
@@ -95,84 +93,7 @@ static char *get_release(void)
        fclose(file);
        free(line);
 
-       line = strdup("Unknown");
-
-       return line;
-}
-
-static char *set_wrapped_app(char *line)
-{
-       char *dline = NULL, *app = NULL, *appfile = NULL, *abs_path = NULL;
-       char delim[] = " '";
-       char app_folder[] = "/usr/share/";
-       int r = 0;
-
-       if (!line)
-               return NULL;
-
-       dline = strdup(line);
-
-       app = strtok(dline, delim);
-       while(app) {
-               if (strcmp(app, "--app") == 0) {
-                       app = strtok(NULL, delim);
-                       break;
-               }
-               app = strtok(NULL, delim);
-       }
-       if (!app)
-               goto cleanup_set_wrapped_app;
-       r = asprintf(&abs_path, "%s%s", app_folder, app);
-       if (r == -1) {
-               abs_path = NULL;
-               goto cleanup_set_wrapped_app;
-       } else if (((unsigned int)r) != strlen(app_folder) + strlen(app)) {
-               goto cleanup_set_wrapped_app;
-       }
-
-       appfile = find_executable(abs_path);
-
-cleanup_set_wrapped_app:
-       free(abs_path);
-       free(dline);
-       return appfile;
-}
-
-/*
- * Scan core dump in case a wrapper was used
- * to run the process and get the actual binary name
- */
-static char *wrapper_scan(char *command)
-{
-       char *line = NULL, *appfile = NULL;
-       FILE *file = NULL;
-
-       file = popen(command, "r");
-       if (!file)
-               return NULL;
-
-       while (!feof(file)) {
-               size_t size = 0;
-               int ret = 0;
-               free(line);
-               ret = getline(&line, &size, file);
-               if (!size)
-                       break;
-               if (ret < 0)
-                       break;
-
-               if (strstr(line, "Core was generated by") &&
-                   strstr(line, "--app")) {
-                       /* attempt to update appfile */
-                       appfile = set_wrapped_app(line);
-                       break;
-               }
-       }
-       if (line)
-               free(line);
-       pclose(file);
-
-       return appfile;
+       return NULL;
 }
 
 /*
@@ -315,30 +236,31 @@ static struct oops *extract_core(char *fullpath, char *appfile)
        struct oops *oops = NULL;
        int ret = 0;
        char *command = NULL, *h1 = NULL, *c1 = NULL, *c2 = NULL, *line = NULL;
-       char *text = NULL, *at = NULL, *coretime = NULL;
+       char *text = NULL, *coretime = NULL;
        char *m1 = NULL, *m2 = NULL;
+       int bt_lines = 0, maps_lines = 0;
        FILE *file = NULL;
        char *badchar = NULL;
        char *release = get_release();
        int parsing_maps = 0;
        struct stat stat_buf;
+       size_t size = 0;
+       ssize_t bytesread = 0;
 
        fprintf(stderr, "+ extract_core() called for %s\n", fullpath);
 
        if (asprintf(&command, "LANG=C gdb --batch -f %s %s -x /etc/corewatcher/gdb.command 2> /dev/null", appfile, fullpath) == -1)
                return NULL;
 
-       if (stat(fullpath, &stat_buf) != -1) {
-               coretime = ctime(&stat_buf.st_mtime);
-       }
-       if (coretime == NULL) {
-               if (asprintf(&coretime, "Unknown\n") == -1)
-                       return NULL;
-       }
+       file = popen(command, "r");
+       free(command);
+       if (!file)
+               fprintf(stderr, "+ gdb failed for %s\n", fullpath);
 
-       if ((at = wrapper_scan(command))) {
-               free(appfile);
-               appfile = at;
+       if (stat(fullpath, &stat_buf) != -1) {
+               coretime = malloc(26);
+               if (coretime)
+                       ctime_r(&stat_buf.st_mtime, coretime);
        }
 
        ret = asprintf(&h1,
@@ -346,45 +268,52 @@ static struct oops *extract_core(char *fullpath, char *appfile)
                       "release: %s\n"
                       "time: %s",
                       appfile,
-                      release,
-                      coretime);
-       free(release);
+                      release ? release : "Unknown",
+                      coretime ? coretime : "Unknown");
+       if (release)
+               free(release);
+       if (coretime)
+               free(coretime);
        if (ret == -1)
-               h1 = strdup("Unknown");
-
-       file = popen(command, "r");
-       if (!file)
-               fprintf(stderr, "+ gdb failed for %s\n", fullpath);
+               return NULL;
 
        while (file && !feof(file)) {
-               size_t size = 0;
-
-               free(line);
-               line = NULL;
-               ret = getline(&line, &size, file);
+               bytesread = getline(&line, &size, file);
                if (!size)
                        break;
-               if (ret == -1)
+               if (bytesread == -1)
                        break;
 
+               /* try to figure out if we're onto the maps output yet */
                if (strncmp(line, "From", 4) == 0) {
                        parsing_maps = 1;
-                       /*continue;*/
                }
-               if (strncmp(line, "No shared libraries loaded at this time.", 40) == 0) {
+               /* maps might not be present */
+               if (strncmp(line, "No shared libraries", 19) == 0) {
                        break;
                }
 
                if (!parsing_maps) { /* parsing backtrace */
                        c2 = c1;
+
+                       /* gdb's backtrace lines start with a line number */
                        if (line[0] != '#')
                                continue;
-fixup:                 /* gdb outputs some 0x1a's which break XML */
-                       badchar = memchr(line, 0x1a, size);
-                       if (badchar) {
-                               *badchar = ' ';
-                               goto fixup;
-                       }
+
+                       /* gdb prints some initial info which may include the
+                        * "#0" line of the backtrace, then prints the
+                        * backtrace in its entirety, leading to a
+                        * duplicate "#0" in our summary if we do do: */
+                       if ((bt_lines == 1) && (strncmp(line, "#0 ", 3) == 0))
+                               continue;
+                       bt_lines++;
+
+                       /* gdb outputs some 0x1a's which break XML */
+                       do {
+                               badchar = memchr(line, 0x1a, bytesread);
+                               if (badchar)
+                                       *badchar = ' ';
+                       } while (badchar);
 
                        if (c1) {
                                c1 = NULL;
@@ -397,6 +326,7 @@ fixup:                      /* gdb outputs some 0x1a's which break XML */
                        }
                } else { /* parsing maps */
                        m2 = m1;
+                       maps_lines++;
                        if (m1) {
                                m1 = NULL;
                                if (asprintf(&m1, "%s        %s", m2, line) == -1)
@@ -412,13 +342,6 @@ fixup:                     /* gdb outputs some 0x1a's which break XML */
                free(line);
        if (file)
                pclose(file);
-       free(command);
-
-       if (!h1)
-               h1 = strdup("Unknown");
-
-       if (!m1)
-               m1 = strdup("        Unknown");
 
        ret = asprintf(&text,
                       "%s"
@@ -426,12 +349,17 @@ fixup:                    /* gdb outputs some 0x1a's which break XML */
                       "%s"
                       "maps: |\n"
                       "%s",
-                      h1, c1, m1);
-       if (ret == -1)
-               text = NULL;
-
+                      h1,
+                      c1 ? c1 : "        Unknown",
+                      m1 ? m1 : "        Unknown");
        free(h1);
-       free(c1);
+       if (c1)
+               free(c1);
+       if (m1)
+               free(m1);
+
+       if (ret == -1)
+               return NULL;
 
        oops = malloc(sizeof(struct oops));
        if (!oops) {
@@ -439,7 +367,7 @@ fixup:                      /* gdb outputs some 0x1a's which break XML */
                return NULL;
        }
        memset(oops, 0, sizeof(struct oops));
-       oops->application = strdup(appfile);
+       oops->application = appfile;
        oops->text = text;
        oops->filename = strdup(fullpath);
        return oops;
@@ -588,7 +516,6 @@ static struct oops *process_common(char *fullpath)
                free(appfile);
                return NULL;
        }
-       free(appfile);
 
        return oops;
 }
index dca9f11..5b11264 100644 (file)
@@ -33,7 +33,6 @@
 #include <sys/stat.h>
 #include <glib.h>
 #include <asm/unistd.h>
-#include <proxy.h>
 #include <curl/curl.h>
 
 #include "corewatcher.h"
@@ -103,8 +102,6 @@ static void print_queue(void)
 static size_t writefunction(void *ptr, size_t size, size_t nmemb, void __attribute((unused)) *stream)
 {
        char *httppost_ret = NULL;
-       char *errstr1 = NULL;
-       char *errstr2 = NULL;
        int ret = 0;
 
        httppost_ret = malloc(size * nmemb + 1);
@@ -118,13 +115,21 @@ static size_t writefunction(void *ptr, size_t size, size_t nmemb, void __attribu
        fprintf(stderr, "%s", httppost_ret);
        fprintf(stderr, "\n\n");
 
-       errstr1 = strstr(httppost_ret, "the server encountered an error");
-       errstr2 = strstr(httppost_ret, "ScannerError at /crash_submit/");
-       if (errstr1 || errstr2)
+       if (strstr(httppost_ret, "the server encountered an error") != NULL) {
                ret = -1;
-       else
-               ret = size * nmemb;
+               goto err;
+       }
+       if (strstr(httppost_ret, "ScannerError at /crash_submit/") != NULL) {
+               ret = -1;
+               goto err;
+       }
+       if (strstr(httppost_ret, "was not found on this server") != NULL) {
+               ret = -1;
+               goto err;
+       }
 
+       ret = size * nmemb;
+err:
        free(httppost_ret);
        return ret;
 }
@@ -168,6 +173,36 @@ char *replace_name(char *filename, char *replace, char *new)
        return newfile;
 }
 
+void report_good_send(int *sentcount, struct oops *oops)
+{
+       char *newfilename = NULL;
+
+       fprintf(stderr, "+ successfully sent %s\n", oops->detail_filename);
+       sentcount++;
+
+       newfilename = replace_name(oops->filename, ".processed", ".submitted");
+       rename(oops->filename, newfilename);
+       free(newfilename);
+
+       g_mutex_lock(&core_status.processing_mtx);
+       remove_name_from_hash(oops->filename, core_status.processing_oops);
+       g_mutex_unlock(&core_status.processing_mtx);
+
+       g_mutex_lock(&bt_mtx);
+       g_hash_table_remove(bt_hash, oops->filename);
+       g_mutex_unlock(&bt_mtx);
+       FREE_OOPS(oops);
+}
+
+void report_fail_send(int *failcount, struct oops *oops, struct oops *requeue_list)
+{
+       fprintf(stderr, "+ requeuing %s\n", oops->detail_filename);
+       failcount++;
+
+       oops->next = requeue_list;
+       requeue_list = oops;
+}
+
 /*
  * Worker thread for submitting backtraces
  *
@@ -176,13 +211,10 @@ char *replace_name(char *filename, char *replace, char *new)
  */
 void *submit_loop(void __unused *unused)
 {
-       int i = 0, n = 0, sentcount, failcount;
+       int i = 0, sentcount, failcount;
        struct oops *oops = NULL;
        struct oops *work_list = NULL;
        struct oops *requeue_list = NULL;
-       char *newfilename = NULL;
-       pxProxyFactory *pf = NULL;
-       char **proxies = NULL;
        int result = 0;
        CURL *handle = NULL;
        struct curl_httppost *post = NULL;
@@ -219,28 +251,22 @@ void *submit_loop(void __unused *unused)
 
                /* net init */
                handle = curl_easy_init();
-               pf = px_proxy_factory_new();
                curl_easy_setopt(handle, CURLOPT_NOBODY, 1);
                curl_easy_setopt(handle, CURLOPT_TIMEOUT, 5);
 
                sentcount = 0;
                failcount = 0;
 
-               /* try to find a good url/proxy combo */
+               /* try to find a good url (curl automagically will use config'd proxies */
                for (i = 0; i < url_count; i++) {
                        curl_easy_setopt(handle, CURLOPT_URL, submit_url[i]);
 
-                       /* use a proxy if one is present */
-                       if (pf) {
-                               proxies = px_proxy_factory_get_proxies(pf, submit_url[i]);
-                               if (proxies)
-                                       curl_easy_setopt(handle, CURLOPT_PROXY, proxies[0]);
-                       }
-
                        /* check the connection before POSTing form */
                        result = curl_easy_perform(handle);
-                       if (result)
+                       if (result) {
+                               fprintf(stderr, "+ unable to contact %s\n", submit_url[i]);
                                continue;
+                       }
 
                        /* have a good url/proxy now...send reports there */
                        while (work_list) {
@@ -260,34 +286,12 @@ void *submit_loop(void __unused *unused)
                                curl_formfree(post);
 
                                if (!result) {
-                                       fprintf(stderr, "+ successfully sent %s\n", oops->detail_filename);
-                                       sentcount++;
-
-                                       newfilename = replace_name(oops->filename, ".processed", ".submitted");
-                                       rename(oops->filename, newfilename);
-                                       free(newfilename);
-
-                                       g_mutex_lock(&core_status.processing_mtx);
-                                       remove_name_from_hash(oops->filename, core_status.processing_oops);
-                                       g_mutex_unlock(&core_status.processing_mtx);
-
-                                       g_mutex_lock(&bt_mtx);
-                                       g_hash_table_remove(bt_hash, oops->filename);
-                                       g_mutex_unlock(&bt_mtx);
-                                       FREE_OOPS(oops);
+                                       report_good_send(&sentcount, oops);
                                } else {
-                                       fprintf(stderr, "+ requeuing %s\n", oops->detail_filename);
-                                       failcount++;
-
-                                       oops->next = requeue_list;
-                                       requeue_list = oops;
+                                       report_fail_send(&failcount, oops, requeue_list);
                                }
                        }
 
-                       for (n = 0; proxies[n]; n++)
-                               free(proxies[n]);
-                       free(proxies);
-
                        openlog("corewatcher", 0, LOG_KERN);
                        if (sentcount)
                                syslog(LOG_WARNING, "Successful sent %d coredump signatures to %s", sentcount, submit_url[i]);
@@ -295,8 +299,11 @@ void *submit_loop(void __unused *unused)
                                syslog(LOG_WARNING, "Failed to send %d coredump signatures to %s", failcount, submit_url[i]);
                        closelog();
                }
+               if (work_list) {
+                       work_list->next = requeue_list;
+                       requeue_list = work_list;
+               }
 
-               px_proxy_factory_free(pf);
                curl_easy_cleanup(handle);
        }