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 <[email protected]> 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/> _______________________________________________ lttng-dev mailing list [email protected] http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
