Hi,
sdiff makes a false assumption about diffargv:
/*
* Allocate memory for diff arguments and NULL.
* Each flag has at most one argument, so doubling argc gives an
* upper limit of how many diff args can be passed. argv[0],
* file1, and file2 won't have arguments so doubling them will
* waste some memory; however we need an extra space for the
* NULL at the end, so it sort of works out.
*/
This is unfortunately not true, because we can easily specify multiple
single-letter arguments in one argument, like:
$ sdiff -aaaaaa<insert a lot of args here> file1 file2
Segmentation fault
The allocation of diffargv should therefore happen dynamically while
parsing arguments.
While doing so, I created another char array, holding all single-letter
arguments that cannot have optargs on their own. This makes sure that we
do not create more diffargv elements than the system could possibly
handle.
Tobias
Index: sdiff.c
===================================================================
RCS file: /cvs/src/usr.bin/sdiff/sdiff.c,v
retrieving revision 1.28
diff -u -p -r1.28 sdiff.c
--- sdiff.c 7 Jun 2009 13:29:50 -0000 1.28
+++ sdiff.c 30 Mar 2013 15:49:53 -0000
@@ -153,67 +153,63 @@ FAIL:
exit(2);
}
+static char **
+addarg(char *arg)
+{
+ static char **diffargv = NULL;
+ static int diffargc = 0;
+
+ diffargv = realloc(diffargv, (diffargc + 1) * sizeof(char **));
+ if (diffargv == NULL)
+ err(2, "diff arguments");
+ diffargv[diffargc++] = arg;
+
+ return diffargv;
+}
+
int
main(int argc, char **argv)
{
FILE *diffpipe, *file1, *file2;
- size_t diffargc = 0, wflag = WIDTH;
+ size_t shortoptslen = 0, wflag = WIDTH;
int ch, fd[2], status;
pid_t pid;
const char *outfile = NULL;
char **diffargv, *diffprog = "diff", *filename1, *filename2,
- *tmp1, *tmp2, *s1, *s2;
+ *shortopts = NULL, *tmp1, *tmp2, *s1, *s2;
/*
* Process diff flags.
*/
- /*
- * Allocate memory for diff arguments and NULL.
- * Each flag has at most one argument, so doubling argc gives an
- * upper limit of how many diff args can be passed. argv[0],
- * file1, and file2 won't have arguments so doubling them will
- * waste some memory; however we need an extra space for the
- * NULL at the end, so it sort of works out.
- */
- if (!(diffargv = calloc(argc, sizeof(char **) * 2)))
- err(2, "main");
/* Add first argument, the program name. */
- diffargv[diffargc++] = diffprog;
+ diffargv = addarg(diffprog);
while ((ch = getopt_long(argc, argv, "aBbdEHI:ilo:stWw:",
longopts, NULL)) != -1) {
const char *errstr;
+ char c = '\0';
switch (ch) {
case 'a':
- diffargv[diffargc++] = "-a";
- break;
case 'B':
- diffargv[diffargc++] = "-B";
- break;
case 'b':
- diffargv[diffargc++] = "-b";
- break;
case 'd':
- diffargv[diffargc++] = "-d";
- break;
case 'E':
- diffargv[diffargc++] = "-E";
+ case 'H':
+ case 'i':
+ case 't':
+ c = ch;
break;
case 'F':
diffargv[0] = diffprog = optarg;
break;
- case 'H':
- diffargv[diffargc++] = "-H";
- break;
case 'I':
Iflag = 1;
- diffargv[diffargc++] = "-I";
- diffargv[diffargc++] = optarg;
+ diffargv = addarg("-I");
+ diffargv = addarg(optarg);
break;
- case 'i':
- diffargv[diffargc++] = "-i";
+ c = 'i';
break;
case 'l':
lflag = 1;
@@ -222,16 +218,13 @@ main(int argc, char **argv)
outfile = optarg;
break;
case 'S':
- diffargv[diffargc++] = "--strip-trailing-cr";
+ diffargv = addarg("--strip-trailing-cr");
break;
case 's':
sflag = 1;
break;
- case 't':
- diffargv[diffargc++] = "-t";
- break;
case 'W':
- diffargv[diffargc++] = "-w";
+ c = 'w';
break;
case 'w':
wflag = strtonum(optarg, WIDTH_MIN,
@@ -243,6 +236,22 @@ main(int argc, char **argv)
usage();
}
+ if (c != '\0') {
+ if (shortopts == NULL) {
+ shortopts = malloc(3 * sizeof(char *));
+ if (shortopts == NULL)
+ errx(2, "diff arguments");
+ shortoptslen = 3;
+ shortopts[0] = '-';
+ } else {
+ shortopts = realloc(shortopts,
+ ++shortoptslen * sizeof(char *));
+ if (shortopts == NULL)
+ errx(2, "diff arguments");
+ }
+ shortopts[shortoptslen - 2] = c;
+ shortopts[shortoptslen - 1] = '\0';
+ }
}
argc -= optind;
argv += optind;
@@ -280,10 +289,12 @@ main(int argc, char **argv)
filename2 = tmp2;
}
- diffargv[diffargc++] = filename1;
- diffargv[diffargc++] = filename2;
+ if (shortopts != NULL)
+ diffargv = addarg(shortopts);
+ diffargv = addarg(filename1);
+ diffargv = addarg(filename2);
/* Add NULL to end of array to indicate end of array. */
- diffargv[diffargc++] = NULL;
+ diffargv = addarg(NULL);
/* Subtract column divider and divide by two. */
width = (wflag - 3) / 2;
