lib: Return instead of exit in trace

Modifies behavior of spdk_trace_init() such that it returns
on failure instead of terminating with exit().

Change-Id: I0ba321cddd233008a4b19bc592728230d89a11b2
Signed-off-by: Lance Hartmann <lance.hartmann@oracle.com>
Reviewed-on: https://review.gerrithub.io/403357
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>

include/spdk/trace.h

@@ -190,8 +190,9 @@ void spdk_trace_set_tpoint_group_mask(uint64_t tpoint_group_mask);
  * human-readable format.
  *
  * \param shm_name Name of shared memory.
+ * \return 0 on success, else non-zero indicates a failure.
  */
-void spdk_trace_init(const char *shm_name);
+int spdk_trace_init(const char *shm_name);
 
 /**
  * Unmap global trace memory structs.

lib/event/app.c

@@ -277,7 +277,7 @@ int
 spdk_app_start(struct spdk_app_opts *opts, spdk_event_fn start_fn,
 	       void *arg1, void *arg2)
 {
-	struct spdk_conf		*config;
+	struct spdk_conf		*config = NULL;
 	struct spdk_conf_section	*sp;
 	char			shm_name[64];
 	int			rc;
@@ -318,13 +318,11 @@ spdk_app_start(struct spdk_app_opts *opts, spdk_event_fn start_fn,
 		rc = spdk_conf_read(config, opts->config_file);
 		if (rc != 0) {
 			SPDK_ERRLOG("Could not read config file %s\n", opts->config_file);
-			spdk_conf_free(config);
+			goto app_start_conf_free_err;
-			return 1;
 		}
 		if (spdk_conf_first_section(config) == NULL) {
 			SPDK_ERRLOG("Invalid config file %s\n", opts->config_file);
-			spdk_conf_free(config);
+			goto app_start_conf_free_err;
-			return 1;
 		}
 	}
 	spdk_conf_set_as_default(config);
@@ -369,9 +367,7 @@ spdk_app_start(struct spdk_app_opts *opts, spdk_event_fn start_fn,
 
 	if (spdk_env_init(&env_opts) < 0) {
 		SPDK_ERRLOG("Unable to initialize SPDK env\n");
-		spdk_log_close();
+		goto app_start_log_close_err;
-		spdk_conf_free(g_spdk_app.config);
-		return 1;
 	}
 
 	SPDK_NOTICELOG("Total cores available: %d\n", spdk_env_get_core_count());
@@ -383,24 +379,25 @@ spdk_app_start(struct spdk_app_opts *opts, spdk_event_fn start_fn,
 	 */
 	if ((rc = spdk_reactors_init(opts->max_delay_us)) != 0) {
 		SPDK_ERRLOG("Invalid reactor mask.\n");
-		spdk_log_close();
+		goto app_start_log_close_err;
-		spdk_conf_free(g_spdk_app.config);
-		return 1;
-	}
-
-	if ((rc = spdk_app_setup_signal_handlers(opts)) != 0) {
-		spdk_conf_free(g_spdk_app.config);
-		spdk_log_close();
-		return 1;
 	}
 
+	/*
+	 * Note the call to spdk_trace_init() is located here
+	 * ahead of spdk_app_setup_signal_handlers().
+	 * That's because there is not an easy/direct clean
+	 * way of unwinding alloc'd resources that can occur
+	 * in spdk_app_setup_signal_handlers().
+	 */
 	if (opts->shm_id >= 0) {
 		snprintf(shm_name, sizeof(shm_name), "/%s_trace.%d", opts->name, opts->shm_id);
 	} else {
 		snprintf(shm_name, sizeof(shm_name), "/%s_trace.pid%d", opts->name, (int)getpid());
 	}
 
-	spdk_trace_init(shm_name);
+	if (spdk_trace_init(shm_name) != 0) {
+		goto app_start_log_close_err;
+	}
 
 	if (opts->tpoint_group_mask == NULL) {
 		sp = spdk_conf_find_section(g_spdk_app.config, "Global");
@@ -424,6 +421,10 @@ spdk_app_start(struct spdk_app_opts *opts, spdk_event_fn start_fn,
 		}
 	}
 
+	if ((rc = spdk_app_setup_signal_handlers(opts)) != 0) {
+		goto app_start_trace_cleanup_err;
+	}
+
 	g_spdk_app.rc = 0;
 	g_init_lcore = spdk_env_get_current_core();
 	g_app_start_fn = start_fn;
@@ -437,6 +438,17 @@ spdk_app_start(struct spdk_app_opts *opts, spdk_event_fn start_fn,
 	spdk_reactors_start();
 
 	return g_spdk_app.rc;
+
+app_start_trace_cleanup_err:
+	spdk_trace_cleanup();
+
+app_start_log_close_err:
+	spdk_log_close();
+
+app_start_conf_free_err:
+	spdk_conf_free(config);
+
+	return 1;
 }
 
 void

lib/trace/trace.c

@@ -37,6 +37,7 @@
 #include "spdk/string.h"
 #include "spdk/trace.h"
 
+static int g_trace_fd = -1;
 static char g_shm_name[64];
 
 static struct spdk_trace_histories *g_trace_histories;
@@ -84,31 +85,30 @@ spdk_trace_record(uint16_t tpoint_id, uint16_t poller_id, uint32_t size,
 	}
 }
 
-void
+int
 spdk_trace_init(const char *shm_name)
 {
-	int trace_fd;
 	int i = 0;
 
 	snprintf(g_shm_name, sizeof(g_shm_name), "%s", shm_name);
 
-	trace_fd = shm_open(shm_name, O_RDWR | O_CREAT, 0600);
+	g_trace_fd = shm_open(shm_name, O_RDWR | O_CREAT, 0600);
-	if (trace_fd == -1) {
+	if (g_trace_fd == -1) {
 		fprintf(stderr, "could not shm_open spdk_trace\n");
 		fprintf(stderr, "errno=%d %s\n", errno, spdk_strerror(errno));
-		exit(EXIT_FAILURE);
+		return 1;
 	}
 
-	if (ftruncate(trace_fd, sizeof(*g_trace_histories)) != 0) {
+	if (ftruncate(g_trace_fd, sizeof(*g_trace_histories)) != 0) {
 		fprintf(stderr, "could not truncate shm\n");
-		exit(EXIT_FAILURE);
+		goto trace_init_err;
 	}
 
 	g_trace_histories = mmap(NULL, sizeof(*g_trace_histories), PROT_READ | PROT_WRITE,
-				 MAP_SHARED, trace_fd, 0);
+				 MAP_SHARED, g_trace_fd, 0);
 	if (g_trace_histories == NULL) {
 		fprintf(stderr, "could not mmap shm\n");
-		exit(EXIT_FAILURE);
+		goto trace_init_err;
 	}
 
 	memset(g_trace_histories, 0, sizeof(*g_trace_histories));
@@ -122,6 +122,16 @@ spdk_trace_init(const char *shm_name)
 	}
 
 	spdk_trace_flags_init();
+
+	return 0;
+
+trace_init_err:
+	close(g_trace_fd);
+	g_trace_fd = -1;
+	shm_unlink(shm_name);
+
+	return 1;
+
 }
 
 void
@@ -130,6 +140,7 @@ spdk_trace_cleanup(void)
 	if (g_trace_histories) {
 		munmap(g_trace_histories, sizeof(struct spdk_trace_histories));
 		g_trace_histories = NULL;
+		close(g_trace_fd);
 	}
 	shm_unlink(g_shm_name);
 }