Clean path searching for binary

Gdb needs an executable and a core so we seek out an executable which
may correspond to the crash.  This was done in a loose way and led to
false matches.  This patch makes the search more strict, adds some debug
message output and handles errors better.  As per the earlier commit,
the "path" is hard coded and with only a single member ("/usr/bin"),
but the code remains to allow it to be a colon separated list of
directories.

Signed-off-by: Tim Pepper <timothy.c.pepper@linux.intel.com>

3 files changed, 89 insertions, 41 deletions

diff --git a/src/coredump.c b/src/coredump.c
index a378eea..6244801 100644
--- a/src/coredump.c
+++ b/src/coredump.c
@@ -135,6 +135,7 @@ static int move_core(char *fullpath, char *extension)
 	size_t prefix_len;
 	DIR *dir = NULL;
 	struct dirent *entry = NULL;
+	int ret = 0;
 
 	if (!fullpath)
 		return -1;
@@ -168,11 +169,15 @@ static int move_core(char *fullpath, char *extension)
 		free(coreprefix);
 		coreprefix = s;
 	} else {
-		goto error;
+		ret = -1;
+		goto out;
 	}
 	dir = opendir(processed_folder);
-	if (!dir)
-		goto error;
+	if (!dir) {
+		ret = -1;
+		goto out;
+	}
+
 	while(1) {
 		entry = readdir(dir);
 		if (!entry || !entry->d_name)
@@ -181,26 +186,27 @@ static int move_core(char *fullpath, char *extension)
 			continue;
 		if (!strstr(entry->d_name, coreprefix))
 			continue;
-		fprintf(stderr, "+ ...ignoring/unlinking %s\n", fullpath);
-		unlink(fullpath);
-		closedir(dir);
-		goto error;
+		break;
 	}
 	closedir(dir);
-
-	if (asprintf(&newpath, "%s%s.%s", processed_folder, corefilename, extension) == -1)
-		goto error;
-
 	free(coreprefix);
+
+	if (asprintf(&newpath, "%s%s.%s", processed_folder, corefilename, extension) == -1) {
+		ret = -1;
+		goto out;
+	}
 	free(corefilename);
+
 	rename(fullpath, newpath);
 	free(newpath);
 	return 0;
 
-error:
+out:
 	free(coreprefix);
 	free(corefilename);
-	return -1;
+	fprintf(stderr, "+ ...move failed, ignoring/unlinking %s\n", fullpath);
+	unlink(fullpath);
+	return ret;
 }
 
 
@@ -420,7 +426,7 @@ static void write_core_detail_file(struct oops *oops)
 static struct oops *process_common(char *fullpath)
 {
 	struct oops *oops = NULL;
-	char *appname = NULL, *appfile = NULL, *corefn = NULL, *reportname = NULL;
+	char *app = NULL, *appname = NULL, *appfile = NULL, *corefn = NULL, *reportname = NULL;
 	struct stat stat_buf;
 
 	corefn = strip_directories(fullpath);
@@ -429,28 +435,39 @@ static struct oops *process_common(char *fullpath)
 		return NULL;
 	}
 
+	/* don't process rpm, gdb or corewatcher crashes */
 	appname = find_causingapp(fullpath);
 	if (!appname) {
+		fprintf(stderr, "+ No appname in %s\n", corefn);
+		move_core(fullpath, "skipped");
 		free(corefn);
-		free(reportname);
 		return NULL;
 	}
-	/*
-	 * don't process rpm, gdb or corewatcher crashes,
-	 * also skip apps which don't appear to be part of the OS
-	 */
-	appfile = find_apppath(appname);
-	if (!appfile ||
-	    !strncmp(appname, "rpm", 3) ||
-	    !strncmp(appname, "gdb", 3) ||
-	    !strncmp(appname, "corewatcher", 11)) {
+	app = strip_directories(appname);
+	if (!app ||
+	    !strncmp(app, "rpm", 3) ||
+	    !strncmp(app, "gdb", 3) ||
+	    !strncmp(app, "corewatcher", 11)) {
+		fprintf(stderr, "+ ...skipping %s's %s\n", app, corefn);
+		move_core(fullpath, "skipped");
 		free(corefn);
 		free(appname);
-		fprintf(stderr, "+ ...ignoring %s's %s\n", appname, fullpath);
+		free(app);
+		return NULL;
+	}
+
+	/* also skip apps which don't appear to be part of the OS */
+	appfile = find_apppath(appname);
+	if (!appfile) {
+		fprintf(stderr, "+ ...skipping %s's %s\n", appname, corefn);
 		move_core(fullpath, "skipped");
+		free(corefn);
+		free(appname);
+		free(app);
 		return NULL;
 	}
 	free(appname);
+	free(app);
 
 	reportname = make_report_filename(corefn);
 	if (!reportname) {
diff --git a/src/corewatcher.h b/src/corewatcher.h
index be82929..cc22c20 100644
--- a/src/corewatcher.h
+++ b/src/corewatcher.h
@@ -73,6 +73,7 @@ extern void *scan_processed_folder(void __unused *unused);
 extern const char *core_folder;
 extern const char *processed_folder;
 extern void enable_corefiles(int diskfree);
+extern char *strip_directories(char *fullpath);
 
 /* configfile.c */
 extern void read_config_file(char *filename);
diff --git a/src/find_file.c b/src/find_file.c
index fe1bf2d..510eeb3 100644
--- a/src/find_file.c
+++ b/src/find_file.c
@@ -15,47 +15,77 @@
 #include <string.h>
 #include <glib.h>
 
-#ifndef PATH_MAX
-#define PATH_MAX 4096
-#endif
-
 #include "corewatcher.h"
 
+/* Attempt to find a file which would match the path/file fragment and
+ * possibly represent a system binary.  We'll get more checking later when
+ * we call gdb.  This is just a best effort candidate executable file to
+ * hand to gdb along with a core. */
 char *find_apppath(char *fragment)
 {
-	char *path = "/usr/bin"; /* system path */
+	char *path = "/usr/bin";         /* ':' sep'd system path */
 	char *c1, *c2;
 	char *filename = NULL;
+	char *candidate = NULL;
+	char *apppath = NULL;
+	int stripped = 0;
 
 	fprintf(stderr, "+ Looking for %s\n", fragment);
 
-	if (fragment == NULL || strlen(fragment) < 3)
+	/* explicit absolute path in the standard directory */
+	if (!strncmp(fragment, "/usr/bin", 8)) {
+		if (!access(fragment, X_OK)) {
+			fprintf(stderr, "+  found system executable %s\n", fragment);
+			return strdup(fragment);
+		}
+		fprintf(stderr, "+  can't access system executable %s\n", fragment);
+		return NULL;
+	}
+
+	/* explicit absolute path not in the standard directory */
+	if (!strncmp(fragment, "/", 1)) {
+		fprintf(stderr, "+  bad absolute path %s\n", fragment);
 		return NULL;
+	}
 
-	/* Deal with absolute paths first */
-	if (!access(fragment, X_OK)) {
-		if (!(filename = strdup(fragment)))
+        /* relative path: problematic as we cannot at this point know the
+	 * $PATH from the environment when the crash'd program started */
+	if (strchr(fragment, '/')) {
+	        candidate = strip_directories(fragment);
+		fprintf(stderr, "+  stripped %s to %s\n", fragment, candidate);
+		if (candidate == NULL)
 			return NULL;
-		return filename;
+		stripped = 1;
+	} else {
+		/* fragment is just an executable's name */
+		candidate = fragment;
 	}
 
+	/* search the path */
 	c1 = path;
 	while (c1 && strlen(c1)>0) {
 		free(filename);
 		filename = NULL;
 		c2 = strchr(c1, ':');
 		if (c2) *c2=0;
-		if(asprintf(&filename, "%s/%s", c1, fragment) == -1)
-			return NULL;
+		fprintf(stderr, "+  looking in %s\n", c1);
+		if(asprintf(&filename, "%s/%s", c1, candidate) == -1) {
+			apppath = NULL;
+			free(filename);
+			goto out;
+		}
 		if (!access(filename, X_OK)) {
-			printf("+ Found %s\n", filename);
-			return filename;
+			printf("+  found %s\n", filename);
+			apppath = filename;
+			goto out;
 		}
 		c1 = c2;
 		if (c2) c1++;
 	}
-	free(filename);
-	return NULL;
+out:
+	if (stripped)
+		free(candidate);
+	return apppath;
 }
 
 char *find_causingapp(char *fullpath)