[lttng-dev] [PATCH] ltt-control "Minimal changes" update as 9 little patches
Thibault, Daniel
Daniel.Thibault at drdc-rddc.gc.ca
Fri Dec 16 11:08:10 EST 2011
----------------------------------------------------------------------
> Date: Thu, 15 Dec 2011 14:18:23 -0500
> From: Yannick Brosseau <yannick.brosseau at gmail.com>
> The format is better, thank you. Would it be possible for you to split
> the patch in several git commit and thus into several different patches.
> (One "logical" change/feature per patch). This would make the review
> much easier, and allow us to comment on specific changes.
>
> Yannick
----------------------------------------------------------------------
Okay, broke it down into nine steps:
0.89.1: Cosmetic fixes
liblttctl.c:
* Comment out the unused #define MAX_CHANNEL
* Cosmetic re-ordering of the includes
* Minor formatting fixes
----------------------------------------------------------------------
diff --git a/liblttctl/liblttctl.c b/liblttctl/liblttctl.c
index 1032f15..e1b6dd5 100644
--- a/liblttctl/liblttctl.c
+++ b/liblttctl/liblttctl.c
@@ -26,16 +26,16 @@
#include <config.h>
#endif
-#include <liblttctl/lttctl.h>
+#include <stdlib.h>
#include <errno.h>
#include <stdio.h>
#include <string.h>
+#include <fcntl.h>
#include <dirent.h>
#include <limits.h>
-#include <fcntl.h>
-#include <stdlib.h>
+#include <liblttctl/lttctl.h>
-#define MAX_CHANNEL (256)
+//#define MAX_CHANNEL (256)
static char debugfsmntdir[PATH_MAX];
@@ -510,6 +510,7 @@
return ret;
}
+
int lttctl_set_channel_overwrite(const char *name, const char *channel,
int overwrite)
{
@@ -577,6 +578,7 @@
return ret;
}
+
int lttctl_set_channel_subbuf_num(const char *name, const char *channel,
unsigned subbuf_num)
{
----------------------------------------------------------------------
0.89.2: Remove unnecessary param check in lttctl_sendop()
lttctl_sendop() is private and all of its callers validate the
parameters beforehand
----------------------------------------------------------------------
diff --git a/liblttctl/liblttctl.c b/liblttctl/liblttctl.c
index e1b6dd5..fa54815 100644
--- a/liblttctl/liblttctl.c
+++ b/liblttctl/liblttctl.c
@@ -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,
----------------------------------------------------------------------
0.89.3: Fix missing malloc() failure detection in lttctl_get_channellist()
malloc() failure in lttctl_get_channellist() detected and reported
appropriately
----------------------------------------------------------------------
diff --git a/liblttctl/liblttctl.c b/liblttctl/liblttctl.c
index fa54815..d94b6aa 100644
--- a/liblttctl/liblttctl.c
+++ b/liblttctl/liblttctl.c
@@ -222,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);
----------------------------------------------------------------------
0.89.4: Fix missing parameter checks in lttctl_set_trans() and getdebugfsmntdir()
Fixes missing parameter checks in lttctl_set_trans() and
getdebugfsmntdir()
----------------------------------------------------------------------
diff --git a/liblttctl/liblttctl.c b/liblttctl/liblttctl.c
index d94b6aa..e78bdbe 100644
--- a/liblttctl/liblttctl.c
+++ b/liblttctl/liblttctl.c
@@ -404,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;
@@ -770,6 +770,9 @@
char mnt_type[PATH_MAX];
int trymount_done = 0;
+ if (!mntdir)
+ return -EINVAL;
+
FILE *fp = fopen("/proc/mounts", "r");
if (!fp)
return -EINVAL;
----------------------------------------------------------------------
0.89.5: Fix memory leaks in lttctl_set_channel_*() functions
Fixes memory leaks in the lttctl_set_channel_*() functions
----------------------------------------------------------------------
diff --git a/liblttctl/liblttctl.c b/liblttctl/liblttctl.c
index e78bdbe..90ec2c5 100644
--- a/liblttctl/liblttctl.c
+++ b/liblttctl/liblttctl.c
@@ -476,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 +542,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;
}
@@ -610,9 +612,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;
}
@@ -678,9 +681,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;
}
@@ -746,9 +750,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;
}
----------------------------------------------------------------------
0.89.6: getdebugfsmntdir() fixes
getdebugfsmntdir():
* Fix missing fclose(FILE *fp)
* Prevent potential string buffer overruns by limiting the
length of the debugfs mounting point path
* Prevent useless re-scan of /proc/mounts/
when the mount command fails
----------------------------------------------------------------------
diff --git a/liblttctl/liblttctl.c b/liblttctl/liblttctl.c
index 90ec2c5..e4a7430 100644
--- a/liblttctl/liblttctl.c
+++ b/liblttctl/liblttctl.c
@@ -788,16 +788,25 @@
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 also for the channel name
+ // This check ensures sprintf of a debugfs path will never overrun its target char x[PATH_MAX]
+ if (strlen(mnt_dir) >= (PATH_MAX - (4 + 9 + 9 + 13 + 2*NAME_MAX))) {
+ fclose(fp);
+ return -ENOENT;
+ }
strcpy(mntdir, mnt_dir);
+ fclose(fp);
return 0;
}
}
if (!trymount_done) {
- mount("debugfs", "/sys/kernel/debug/", "debugfs", 0, NULL);
trymount_done = 1;
- goto find_again;
+ if (!mount("debugfs", "/sys/kernel/debug/", "debugfs", 0, NULL))
+ goto find_again;
}
-
+ fclose(fp);
return -ENOENT;
}
----------------------------------------------------------------------
0.89.7: Cosmetic fixes
lttctl.c:
* Cosmetic re-ordering of the #includes
* Minor formatting fixes
----------------------------------------------------------------------
diff --git a/lttctl/lttctl.c b/lttctl/lttctl.c
index e77b7f3..57cf164 100644
--- a/lttctl/lttctl.c
+++ b/lttctl/lttctl.c
@@ -28,16 +28,16 @@
#include <config.h>
#endif
-#include <liblttctl/lttctl.h>
-#include <errno.h>
+#define _GNU_SOURCE
#include <stdio.h>
#include <stdlib.h>
-#include <sys/wait.h>
-#include <unistd.h>
+#include <errno.h>
#include <string.h>
+#include <unistd.h>
#include <limits.h>
-#define _GNU_SOURCE
#include <getopt.h>
+#include <sys/wait.h>
+#include <liblttctl/lttctl.h>
#define OPT_MAX (1024)
#define OPT_NAMELEN (256)
@@ -135,10 +135,10 @@
printf(" channel.<channelname>.overwrite=\n");
printf(" channel.<channelname>.bufnum=\n");
printf(" channel.<channelname>.bufsize= (in bytes, rounded to "
- "next power of 2)\n");
+ "next power of 2)\n");
printf(" <channelname> can be set to all for all channels\n");
printf(" channel.<channelname>.switch_timer= (timer interval in "
- "ms)\n");
+ "ms)\n");
printf("\n");
printf(" Integration options:\n");
printf(" -C, --create_start\n");
@@ -154,7 +154,7 @@
printf(" Number of lttd threads, For -w option\n");
printf(" --channel_root PATH\n");
printf(" Set channels root path, For -w option."
- " (ex. /mnt/debugfs/ltt)\n");
+ " (ex. /mnt/debugfs/ltt)\n");
printf("\n");
printf(" Environment variables:\n");
printf(" LTT_DAEMON\n");
@@ -268,10 +268,10 @@
return -EINVAL;
}
if (opt_valstr[0] == 'Y' || opt_valstr[0] == 'y'
- || opt_valstr[0] == '1')
+ || opt_valstr[0] == '1')
opt_val = 1;
else if (opt_valstr[0] == 'N' || opt_valstr[0] == 'n'
- || opt_valstr[0] == '0')
+ || opt_valstr[0] == '0')
opt_val = 0;
else {
return -EINVAL;
@@ -284,10 +284,10 @@
return -EINVAL;
}
if (opt_valstr[0] == 'Y' || opt_valstr[0] == 'y'
- || opt_valstr[0] == '1')
+ || opt_valstr[0] == '1')
opt_val = 1;
else if (opt_valstr[0] == 'N' || opt_valstr[0] == 'n'
- || opt_valstr[0] == '0')
+ || opt_valstr[0] == '0')
opt_val = 0;
else {
return -EINVAL;
@@ -378,7 +378,7 @@
if (!strcmp("channel", name1)) {
opt = find_insert_channel_opt(name2);
if ((ret = set_channel_opt(&opt->opt_mode.chan_opt,
- name3, opt_valstr) != 0)) {
+ name3, opt_valstr) != 0)) {
fprintf(stderr, "Option name error2: %s\n", optarg);
return ret;
}
@@ -402,20 +402,20 @@
int ret = 0;
static struct option longopts[] = {
- {"create", no_argument, NULL, 'c'},
- {"destroy", no_argument, NULL, 'd'},
- {"start", no_argument, NULL, 's'},
- {"pause", no_argument, NULL, 'p'},
- {"help", no_argument, NULL, 'h'},
+ {"create", no_argument, NULL, 'c'},
+ {"destroy", no_argument, NULL, 'd'},
+ {"start", no_argument, NULL, 's'},
+ {"pause", no_argument, NULL, 'p'},
+ {"help", no_argument, NULL, 'h'},
{"transport", required_argument, NULL, 2},
- {"option", required_argument, NULL, 'o'},
+ {"option", required_argument, NULL, 'o'},
{"create_start", no_argument, NULL, 'C'},
{"pause_destroy", no_argument, NULL, 'D'},
- {"write", required_argument, NULL, 'w'},
- {"append", no_argument, NULL, 'a'},
+ {"write", required_argument, NULL, 'w'},
+ {"append", no_argument, NULL, 'a'},
{"dump_threads", required_argument, NULL, 'n'},
{"channel_root", required_argument, NULL, 3},
- { NULL, 0, NULL, 0 },
+ { NULL, 0, NULL, 0 },
};
/*
@@ -629,31 +629,32 @@
if (opt->enable != -1) {
if ((ret = lttctl_set_channel_enable(opt_tracename,
- opt->chan_name,
- opt->enable)) != 0)
+ opt->chan_name,
+ opt->enable)) != 0)
return ret;
}
if (opt->overwrite != -1) {
if ((ret = lttctl_set_channel_overwrite(opt_tracename,
- opt->chan_name,
- opt->overwrite)) != 0)
+ opt->chan_name,
+ opt->overwrite)) != 0)
return ret;
}
if (opt->bufnum != -1) {
if ((ret = lttctl_set_channel_subbuf_num(opt_tracename,
- opt->chan_name,
- opt->bufnum)) != 0)
+ opt->chan_name,
+ opt->bufnum)) != 0)
return ret;
}
if (opt->bufsize != -1) {
if ((ret = lttctl_set_channel_subbuf_size(opt_tracename,
- opt->chan_name,
- opt->bufsize)) != 0)
+ opt->chan_name,
+ opt->bufsize)) != 0)
return ret;
}
if (opt->switch_timer != -1) {
if ((ret = lttctl_set_channel_switch_timer(opt_tracename,
- opt->chan_name, opt->switch_timer)) != 0)
+ opt->chan_name,
+ opt->switch_timer)) != 0)
return ret;
}
----------------------------------------------------------------------
0.89.8: Make opt_head, last_opt and set_channel_opt() private
lttctl.c:
* Make opt_head, last_opt and set_channel_opt() private
----------------------------------------------------------------------
diff --git a/lttctl/lttctl.c b/lttctl/lttctl.c
index 57cf164..e410585 100644
--- a/lttctl/lttctl.c
+++ b/lttctl/lttctl.c
@@ -64,7 +64,7 @@
struct lttctl_option *next;
};
-struct lttctl_option *opt_head, *last_opt;
+static struct lttctl_option *opt_head, *last_opt;
static int opt_create;
static int opt_destroy;
@@ -259,7 +259,7 @@
return new_opt;
}
-int set_channel_opt(struct channel_option *opt, char *opt_name, char *opt_valstr)
+static int set_channel_opt(struct channel_option *opt, char *opt_name, char *opt_valstr)
{
int opt_val, ret;
----------------------------------------------------------------------
0.89.9: Detect malloc() failures in find_insert_channel_opt()
lttctl.c:
* Fix missing malloc() failure detections in find_insert_channel_opt()
* Handle resulting -ENOMEM in parst_opt()
----------------------------------------------------------------------
diff --git a/lttctl/lttctl.c b/lttctl/lttctl.c
index e410585..8af9fda 100644
--- a/lttctl/lttctl.c
+++ b/lttctl/lttctl.c
@@ -236,10 +236,12 @@
if (!opt_head) {
opt_head = (struct lttctl_option *)malloc(sizeof(struct lttctl_option));
- init_channel_opt(&opt_head->opt_mode.chan_opt, opt_name);
- opt_head->type = CHANNEL;
- opt_head->next = NULL;
- last_opt = opt_head;
+ if (opt_head) {
+ init_channel_opt(&opt_head->opt_mode.chan_opt, opt_name);
+ opt_head->type = CHANNEL;
+ opt_head->next = NULL;
+ last_opt = opt_head;
+ }
return opt_head;
}
@@ -251,11 +253,13 @@
}
new_opt = (struct lttctl_option *)malloc(sizeof(struct lttctl_option));
- init_channel_opt(&new_opt->opt_mode.chan_opt, opt_name);
- new_opt->type = CHANNEL;
- new_opt->next = NULL;
- last_opt->next = new_opt;
- last_opt = new_opt;
+ if (new_opt) {
+ init_channel_opt(&new_opt->opt_mode.chan_opt, opt_name);
+ new_opt->type = CHANNEL;
+ new_opt->next = NULL;
+ last_opt->next = new_opt;
+ last_opt = new_opt;
+ }
return new_opt;
}
@@ -377,6 +381,7 @@
if (!strcmp("channel", name1)) {
opt = find_insert_channel_opt(name2);
+ if (!opt) return -ENOMEM; //malloc failure
if ((ret = set_channel_opt(&opt->opt_mode.chan_opt,
name3, opt_valstr) != 0)) {
fprintf(stderr, "Option name error2: %s\n", optarg);
----------------------------------------------------------------------
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