[lttng-dev] [PATCH] A minimal rewrite of lttctl (additional)

[Thibault, Daniel]

   Just realized the liblttctl.c patch is a little bit off.  Here is the correct patch, followed by a recap of what I missed last time.

--------------------
--- ../ltt-control-0.89-792f03c/liblttctl/liblttctl.c	2011-05-12 08:44:27.000000000 -0400
+++ ../ltt-control-0.89-792f03c.minimalchanges/liblttctl/liblttctl.c	2011-12-07 15:55:24.000000000 -0500
@@ -35,7 +35,7 @@
 #include <fcntl.h>
 #include <stdlib.h>
 
-#define MAX_CHANNEL	(256)
+//#define MAX_CHANNEL	(256)
 
 static char debugfsmntdir[PATH_MAX];
 
@@ -107,11 +107,6 @@
 {
 	int fd;
 
-	if (!fname) {
-		fprintf(stderr, "%s: args invalid\n", __func__);
-		return 1;
-	}
-
 	fd = open(fname, O_WRONLY);
 	if (fd == -1) {
 		fprintf(stderr, "%s: open %s failed: %s\n", __func__, fname,
@@ -227,6 +222,10 @@
 			continue;
 		old_list = list;
 		list = malloc(sizeof(char *) * ++nr_chan);
+		if (!list) {
+			nr_chan = -ENOMEM;
+			break;
+		}
 		memcpy(list, old_list, sizeof(*list) * (nr_chan - 1));
 		free(old_list);
 		list[nr_chan - 1] = strdup(dirent->d_name);
@@ -405,7 +404,7 @@
 	int ret;
 	char ctlfname[PATH_MAX];
 
-	if (!name) {
+	if (!name || !trans) {
 		fprintf(stderr, "%s: args invalid\n", __func__);
 		ret = -EINVAL;
 		goto arg_error;
@@ -477,9 +476,10 @@
 			goto op_err;
 		}
 
-		for (; n_channel > 0; n_channel--) {
+		int ch = 0;
+		for ( ; ch < n_channel; ch++) {
 			ret = __lttctl_set_channel_enable(name,
-				channellist[n_channel - 1], enable);
+				channellist[ch], enable);
 			if (ret)
 				goto op_err_clean;
 		}
@@ -541,9 +541,10 @@
 			goto op_err;
 		}
 
-		for (; n_channel > 0; n_channel--) {
+		int ch = 0;
+		for ( ; ch < n_channel; ch++) {
 			ret = __lttctl_set_channel_overwrite(name,
-				channellist[n_channel - 1], overwrite);
+				channellist[ch], overwrite);
 			if (ret)
 				goto op_err_clean;
 		}
@@ -609,9 +610,10 @@
 			goto op_err;
 		}
 
-		for (; n_channel > 0; n_channel--) {
+		int ch = 0;
+		for ( ; ch < n_channel; ch++) {
 			ret = __lttctl_set_channel_subbuf_num(name,
-				channellist[n_channel - 1], subbuf_num);
+				channellist[ch], subbuf_num);
 			if (ret)
 				goto op_err_clean;
 		}
@@ -677,9 +679,10 @@
 			goto op_err;
 		}
 
-		for (; n_channel > 0; n_channel--) {
+		int ch = 0;
+		for ( ; ch < n_channel; ch++) {
 			ret = __lttctl_set_channel_subbuf_size(name,
-				channellist[n_channel - 1], subbuf_size);
+				channellist[ch], subbuf_size);
 			if (ret)
 				goto op_err_clean;
 		}
@@ -745,9 +748,10 @@
 			goto op_err;
 		}
 
-		for (; n_channel > 0; n_channel--) {
+		int ch = 0;
+		for ( ; ch < n_channel; ch++) {
 			ret = __lttctl_set_channel_switch_timer(name,
-				channellist[n_channel - 1], switch_timer);
+				channellist[ch], switch_timer);
 			if (ret)
 				goto op_err_clean;
 		}
@@ -769,6 +773,9 @@
 	char mnt_type[PATH_MAX];
 	int trymount_done = 0;
 
+	if (!mntdir)
+		return -EINVAL;
+
 	FILE *fp = fopen("/proc/mounts", "r");
 	if (!fp)
 		return -EINVAL;
@@ -779,16 +786,24 @@
 			break;
 
 		if (!strcmp(mnt_type, "debugfs")) {
+			// 4 for the LTT_PATH "/ltt", 9 for the LTT_CONTROL_PATH "/control/"
+			// 9 for the LTT_CHANNEL_PATH "/channel/", 13 for the LTT_BUFFERS_TIMER_PATH "/switch_timer"
+			// NAME_MAX for the trace name and the channel name
+			if (strlen(mnt_dir) >= (PATH_MAX - (4 + 9 + 9 + 13 + 2*NAME_MAX))) {
+				close(fp);
+				return -ENOENT;
+			}
 			strcpy(mntdir, mnt_dir);
+			close(fp);
 			return 0;
 		}
 	}
 
 	if (!trymount_done) {
-		mount("debugfs", "/sys/kernel/debug/", "debugfs", 0, NULL);
 		trymount_done = 1;
+		if (!mount("debugfs", "/sys/kernel/debug/", "debugfs", 0, NULL))
 		goto find_again;
 	}
-
+	close(fp);
 	return -ENOENT;
}
--------------------

   To recap, getdebugfsmntdir() was not closing the open FILE* handle fp before returning (three additional lines).

--------------------
@@ -779,16 +786,24 @@
 			break;
 
 		if (!strcmp(mnt_type, "debugfs")) {
+			// 4 for the LTT_PATH "/ltt", 9 for the LTT_CONTROL_PATH "/control/"
+			// 9 for the LTT_CHANNEL_PATH "/channel/", 13 for the LTT_BUFFERS_TIMER_PATH "/switch_timer"
+			// NAME_MAX for the trace name and the channel name
+			if (strlen(mnt_dir) >= (PATH_MAX - (4 + 9 + 9 + 13 + 2*NAME_MAX))) {
+				close(fp);
+				return -ENOENT;
+			}
 			strcpy(mntdir, mnt_dir);
+			close(fp);
 			return 0;
 		}
 	}
 
 	if (!trymount_done) {
-		mount("debugfs", "/sys/kernel/debug/", "debugfs", 0, NULL);
 		trymount_done = 1;
+		if (!mount("debugfs", "/sys/kernel/debug/", "debugfs", 0, NULL))
 		goto find_again;
 	}
-
+	close(fp);
 	return -ENOENT;
}
--------------------

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/>