[lttng-dev] [PATCH] ltt-control "Minimal changes" update (resubmission)
Thibault, Daniel
Daniel.Thibault at drdc-rddc.gc.ca
Wed Dec 14 16:31:27 EST 2011
Using Eclipse's EGit plug-in (a standard feature since Eclipse 3.7, it turns out), I generated the patch which follows (as requested by Yannick on 8 December). I think it is produced directly in the git send-email format; correct me if I'm wrong.
As I wrote in my local repository commit message (but had to trim here because it makes the Subject line way too long), this "Minimal changes" update does the following:
* lttctl
** Makes opt_head, last_opt and set_channel_opt() private
** Fixes missing malloc failure detection in find_insert_channel_opt()
** Cosmetic re-ordering of the #includes
** Minor space vs. tab formatting fixes
* liblttctl
** Comments out the unused #define MAX_CHANNEL
** Removes unnecessary lttctl_sendop() parameter check
** Fixes missing malloc failure detection in lttctl_get_channellist()
** Fixes missing parameter check in lttctl_set_trans() and getdebugfsmntdir()
** Fixes memory leak in lttctl_set_channel_*() functions
** Fixes missing fclose(FILE *fp) in getdebugfsmntdir()
** Prevents potential string buffer overruns by limiting the length of the debugfs mounting point path
** Prevents getdebugfsmntdir() from re-scanning /proc/mounts/ uselessly when the mount command fails
** Minor space vs. tab formatting fixes
----------------------------------------------------------------------
>From 6266f28621246f1dda00c2e0cbd391e0169acdcd Wed, 14 Dec 2011 15:44:16 -0500
From: Daniel U. Thibault <daniel.thibault at drdc-rddc.gc.ca>
Date: Wed, 14 Dec 2011 13:33:45 -0500
Subject: [PATCH] "Minimal changes" update
diff --git a/liblttctl/liblttctl.c b/liblttctl/liblttctl.c
index 1032f15..9d4cc62 100644
--- a/liblttctl/liblttctl.c
+++ b/liblttctl/liblttctl.c
@@ -35,7 +35,7 @@
#include <fcntl.h>
#include <stdlib.h>
-#define MAX_CHANNEL (256)
+//#define MAX_CHANNEL (256)
static char debugfsmntdir[PATH_MAX];
@@ -106,11 +106,6 @@
static int lttctl_sendop(const char *fname, const char *op)
{
int fd;
-
- if (!fname) {
- fprintf(stderr, "%s: args invalid\n", __func__);
- return 1;
- }
fd = open(fname, O_WRONLY);
if (fd == -1) {
@@ -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,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;
}
diff --git a/lttctl/lttctl.c b/lttctl/lttctl.c
index e77b7f3..5aadcd0 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)
@@ -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;
@@ -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");
@@ -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,15 +253,17 @@
}
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;
}
-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;
@@ -268,10 +272,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 +288,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;
@@ -377,8 +381,9 @@
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)) {
+ name3, opt_valstr) != 0)) {
fprintf(stderr, "Option name error2: %s\n", optarg);
return ret;
}
@@ -402,20 +407,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,8 +634,8 @@
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) {
@@ -641,19 +646,20 @@
}
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;
}
----------------------------------------------------------------------
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