From: Tim Pepper Date: Thu, 4 Oct 2012 21:45:48 +0000 (-0700) Subject: v0.9.6 bug fix release X-Git-Tag: 2.1b_release~14 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=1bb4db3254caea2028c0eef68d441f834a9d5893;p=external%2Fcorewatcher.git v0.9.6 bug fix release 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 --- diff --git a/configure.ac b/configure.ac index 96c1700..fbcbde1 100644 --- a/configure.ac +++ b/configure.ac @@ -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]) diff --git a/src/Makefile.am b/src/Makefile.am index 3a449e4..af9b2c2 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -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} diff --git a/src/coredump.c b/src/coredump.c index 2990cc7..3bc7540 100644 --- a/src/coredump.c +++ b/src/coredump.c @@ -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; } diff --git a/src/submit.c b/src/submit.c index dca9f11..5b11264 100644 --- a/src/submit.c +++ b/src/submit.c @@ -33,7 +33,6 @@ #include #include #include -#include #include #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); }