Is it possible for you to use git format-patch and git send-email ?
That would make the patch easier to follow
I've added some comments in the patch bellow
On 2011-12-07 16:02, Thibault, Daniel wrote:
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)
Please don't leave commented out code.
Also, why do you remove it?
static char debugfsmntdir[PATH_MAX];
@@ -107,11 +107,6 @@
{
int fd;
- if (!fname) {
- fprintf(stderr, "%s: args invalid\n", __func__);
- return 1;
- }
-
I don't see why you remove this code.
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))
Please indent the code correctly
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))) {
You do not explain the 2*NAME_MAX.
Also, why do you add this check?
+ 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;
Indentation 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/>
_______________________________________________
lttng-dev mailing list
[email protected]
http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
_______________________________________________
lttng-dev mailing list
[email protected]
http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
