[lttng-dev] [PATCH] list.c : Document and enforce return values, fix multiple memory leaks

Thibault, Daniel Daniel.Thibault at drdc-rddc.gc.ca
Thu Feb 2 13:03:57 EST 2012


>From 3808cbb0aaeedd686b1268fee31ca8eafe5fa46d Thu, 2 Feb 2012 13:01:55 -0500
From: Daniel U. Thibault <daniel.thibault at drdc-rddc.gc.ca>
Date: Thu, 2 Feb 2012 13:01:27 -0500
Subject: [PATCH] list.c : Document and enforce return values, fix multiple memory leaks

Signed-off-by: Daniel U. Thibault <daniel.thibault at drdc-rddc.gc.ca>

diff --git a/src/bin/lttng/commands/list.c b/src/bin/lttng/commands/list.c
index f915a16..a73ba3a 100644
--- a/src/bin/lttng/commands/list.c
+++ b/src/bin/lttng/commands/list.c
@@ -115,6 +115,8 @@
 	ret = fread(cmdline, 1, PATH_MAX, fp);
 	if (ret < 0) {
 		perror("fread proc list");
+		free(cmdline);
+		cmdline = NULL;
 	}
 	fclose(fp);
 
@@ -237,15 +239,17 @@
 }
 
 /*
- * Ask session daemon for all user space tracepoints available.
+ * Ask session daemon for all userspace tracepoints available.
+ * Returns the count of events listed (0+), or a negative error code.
  */
 static int list_ust_events(void)
 {
 	int i, size;
 	struct lttng_domain domain;
 	struct lttng_handle *handle;
-	struct lttng_event *event_list;
+	struct lttng_event *event_list = NULL;
 	pid_t cur_pid = 0;
+	char *cmdline;
 
 	DBG("Getting UST tracing events");
 
@@ -253,13 +257,14 @@
 
 	handle = lttng_create_handle(NULL, &domain);
 	if (handle == NULL) {
+		size = -ENOMEM;
 		goto error;
 	}
 
 	size = lttng_list_tracepoints(handle, &event_list);
 	if (size < 0) {
 		ERR("Unable to list UST events");
-		return size;
+		goto error;
 	}
 
 	MSG("UST events:\n-------------");
@@ -271,30 +276,36 @@
 	for (i = 0; i < size; i++) {
 		if (cur_pid != event_list[i].pid) {
 			cur_pid = event_list[i].pid;
-			MSG("\nPID: %d - Name: %s", cur_pid, get_cmdline_by_pid(cur_pid));
+			cmdline = get_cmdline_by_pid(cur_pid);
+			MSG("\nPID: %d - Name: %s", cur_pid, cmdline);
+			free(cmdline);
 		}
 		print_events(&event_list[i]);
 	}
 
 	MSG("");
 
-	free(event_list);
-
-	return CMD_SUCCESS;
+	if (event_list != NULL) {
+		free(event_list);
+	}
 
 error:
-	return -1;
+	if (handle != NULL) {
+		lttng_destroy_handle(handle);
+	}
+	return size;
 }
 
 /*
  * Ask for all trace events in the kernel and pretty print them.
+ * Returns the count of events listed (0+), or a negative error code.
  */
 static int list_kernel_events(void)
 {
 	int i, size;
 	struct lttng_domain domain;
 	struct lttng_handle *handle;
-	struct lttng_event *event_list;
+	struct lttng_event *event_list = NULL;
 
 	DBG("Getting kernel tracing events");
 
@@ -302,16 +313,21 @@
 
 	handle = lttng_create_handle(NULL, &domain);
 	if (handle == NULL) {
+		size = -ENOMEM;
 		goto error;
 	}
 
 	size = lttng_list_tracepoints(handle, &event_list);
 	if (size < 0) {
 		ERR("Unable to list kernel events");
-		return size;
+		goto error;
 	}
 
 	MSG("Kernel events:\n-------------");
+
+	if (size == 0) {
+		MSG("None");
+	}
 
 	for (i = 0; i < size; i++) {
 		print_events(&event_list[i]);
@@ -319,32 +335,34 @@
 
 	MSG("");
 
-	free(event_list);
-
-	return CMD_SUCCESS;
+	if (event_list != NULL) {
+		free(event_list);
+	}
 
 error:
-	return -1;
+	if (handle != NULL) {
+		lttng_destroy_handle(handle);
+	}
+	return size;
 }
 
 /*
  * List events of channel of session and domain.
+ * Returns the count of events listed (0+), or a negative error code.
  */
 static int list_events(const char *channel_name)
 {
-	int ret, count, i;
+	int count, i;
 	struct lttng_event *events = NULL;
 
 	count = lttng_list_events(handle, channel_name, &events);
 	if (count < 0) {
-		ret = count;
 		goto error;
 	}
 
 	MSG("\n%sEvents:", indent4);
 	if (count == 0) {
-		MSG("%sNone\n", indent6);
-		goto end;
+		MSG("%sNone", indent6);
 	}
 
 	for (i = 0; i < count; i++) {
@@ -353,14 +371,12 @@
 
 	MSG("");
 
-end:
 	if (events) {
 		free(events);
 	}
-	ret = CMD_SUCCESS;
 
 error:
-	return ret;
+	return count;
 }
 
 /*
@@ -390,10 +406,11 @@
  * List channel(s) of session and domain.
  *
  * If channel_name is NULL, all channels are listed.
+ * Returns the count of channels listed (0+), or a negative error code.
  */
 static int list_channels(const char *channel_name)
 {
-	int count, i, ret = CMD_SUCCESS;
+	int count, i, ret;
 	unsigned int chan_found = 0;
 	struct lttng_channel *channels = NULL;
 
@@ -401,10 +418,9 @@
 
 	count = lttng_list_channels(handle, &channels);
 	if (count < 0) {
-		ret = count;
 		goto error_channels;
 	} else if (count == 0) {
-		ERR("Channel %s not found", channel_name);
+		ERR("Channel %s not found", (channel_name ? channel_name : "<all>"));
 		goto error;
 	}
 
@@ -425,7 +441,8 @@
 		/* Listing events per channel */
 		ret = list_events(channels[i].name);
 		if (ret < 0) {
-			MSG("%s", lttng_strerror(ret));
+			/* FIXME Shouldn't this error break the loop and percolate up? */
+			ERR("%s", lttng_strerror(ret));
 		}
 
 		if (chan_found) {
@@ -435,33 +452,43 @@
 
 	if (!chan_found && channel_name != NULL) {
 		ERR("Channel %s not found", channel_name);
+		switch (handle->domain.type) {
+		case LTTNG_DOMAIN_KERNEL:
+			count = -LTTCOMM_KERN_CHAN_NOT_FOUND;
+			break;
+		case LTTNG_DOMAIN_UST:
+			count = -LTTCOMM_UST_CHAN_NOT_FOUND;
+			break;
+		default:
+			count = -LTTCOMM_UNKNOWN_DOMAIN;
+			break;
+		}
+
 		goto error;
 	}
-
-	ret = CMD_SUCCESS;
 
 error:
 	free(channels);
 
 error_channels:
-	return ret;
+	return count;
 }
 
 /*
  * List available tracing session. List only basic information.
  *
  * If session_name is NULL, all sessions are listed.
+ * Returns the count of sessions listed (0+), or a negative error code.
  */
 static int list_sessions(const char *session_name)
 {
-	int ret, count, i;
+	int count, i;
 	unsigned int session_found = 0;
-	struct lttng_session *sessions;
+	struct lttng_session *sessions = NULL;
 
 	count = lttng_list_sessions(&sessions);
 	DBG("Session count %d", count);
 	if (count < 0) {
-		ret = count;
 		goto error;
 	}
 
@@ -470,56 +497,46 @@
 	}
 
 	for (i = 0; i < count; i++) {
-		if (session_name != NULL) {
-			if (strncmp(sessions[i].name, session_name, NAME_MAX) == 0) {
-				session_found = 1;
-				MSG("Tracing session %s:%s", session_name, active_string(sessions[i].enabled));
-				MSG("%sTrace path: %s\n", indent4, sessions[i].path);
-				break;
-			}
-			continue;
-		}
-
-		MSG("  %d) %s (%s)%s", i + 1, sessions[i].name, sessions[i].path, active_string(sessions[i].enabled));
-
-		if (session_found) {
+		if (session_name == NULL) {
+			MSG("  %d) %s (%s)%s", i + 1, sessions[i].name, sessions[i].path, active_string(sessions[i].enabled));
+		} else if (strncmp(sessions[i].name, session_name, NAME_MAX) == 0) {
+			session_found = 1;
+			MSG("Tracing session %s:%s", session_name, active_string(sessions[i].enabled));
+			MSG("%sTrace path: %s\n", indent4, sessions[i].path);
+			count = 1;
 			break;
 		}
 	}
 
 	free(sessions);
 
-	if (!session_found && session_name != NULL) {
-		ERR("Session %s not found", session_name);
-	}
-
 	if (session_name == NULL) {
 		MSG("\nUse lttng list <session_name> for more details");
+	} else if (!session_found) {
+		ERR("Session %s not found", session_name);
+		count = -LTTCOMM_SESS_NOT_FOUND;
 	}
 
-	return CMD_SUCCESS;
-
 error:
-	return ret;
+	return count;
 }
 
 /*
  * List available domain(s) for a session.
+ * Returns the count of domains listed (0+), or a negative error code.
  */
 static int list_domains(const char *session_name)
 {
-	int i, count, ret = CMD_SUCCESS;
+	int i, count;
 	struct lttng_domain *domains = NULL;
 
 	MSG("Domains:\n-------------");
 
 	count = lttng_list_domains(session_name, &domains);
 	if (count < 0) {
-		ret = count;
 		goto error;
 	} else if (count == 0) {
 		MSG("  None");
-		goto end;
 	}
 
 	for (i = 0; i < count; i++) {
@@ -531,19 +548,23 @@
 			MSG("  - UST global");
 			break;
 		default:
+			/* FIXME Issue LTTCOMM_UNKNOWN_DOMAIN error? */
+			MSG("  - (Unknown)");
 			break;
 		}
 	}
 
-end:
-	free(domains);
+	if (domains != NULL) {
+		free(domains);
+	}
 
 error:
-	return ret;
+	return count;
 }
 
 /*
  * The 'list <options>' first level command
+ * Returns a CMD_* result value.
  */
 int cmd_list(int argc, const char **argv)
 {
@@ -583,13 +604,14 @@
 
 	/* Get session name (trailing argument) */
 	session_name = poptGetArg(pc);
-	DBG2("Session name: %s", session_name);
+	DBG2("Session name: %s", session_name ? session_name : "<none>");
 
 	if (opt_kernel) {
 		domain.type = LTTNG_DOMAIN_KERNEL;
 	} else if (opt_userspace) {
 		DBG2("Listing userspace global domain");
 		domain.type = LTTNG_DOMAIN_UST;
+	/* TODO As LTTNG_DOMAIN_* values are added, capture them here */
 	}
 
 	handle = lttng_create_handle(session_name, &domain);
@@ -682,6 +704,7 @@
 			}
 		}
 	}
+	ret = CMD_SUCCESS;
 
 end:
 	if (domains) {
------------------------------

Daniel U. Thibault
R & D pour la défense Canada - Valcartier (RDDC Valcartier) / Defence R&D Canada - Valcartier (DRDC Valcartier)
Système de systèmes (SdS) / System of Systems (SoS)
Solutions informatiques et expérimentations (SIE) / Computing Solutions and Experimentations (CSE)
2459 Boul. Pie XI Nord
Québec, QC  G3J 1X5
CANADA
Vox : (418) 844-4000 x4245
Fax : (418) 844-4538
NAC: 918V QSDJ
Gouvernement du Canada / Government of Canada
<http://www.valcartier.drdc-rddc.gc.ca/>



More information about the lttng-dev mailing list