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 <timothy.c.pepper@linux.intel.com>

4 files changed, 116 insertions, 193 deletions

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 <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);
 	}