I've discovered a number of problems with the eval_options() function
(see below). The attached patch implements fixes for them all. Andrew,
let me know if you see any problems with it. If not, I'll commit it
later today.
Btw., as an FYI, C99 strtol() is declared to take a couple of restrict
pointers:
long int strtol(const char * restrict nptr,
char ** restrict endptr,
int base);
which means that the behavior of the function is undefined when
(nptr == &nptr) is true. I fixed this as well.
Martin
1. Undefined behavior for "-t"
$ ./exec -t
Segmentation Fault (core dumped)
2. Negative argument to -t not diagnosed:
$ ./exec -t -1; echo $?
0
3. Poor diagnostic of invalid argument to -t:
$ ./exec -t 1x
./exec: Unknown value for -t: x
4. Abort on missing option argument:
$ ./exec --exit
Assertion failed: 0 != opt, file /build/sebor/stdcxx/util/cmdopt.cpp,
line 141
Abort (core dumped)
5. Missing text for invalid argument:
$ ./exec --exit=9999999999
./exec: Unknown value for --exit:
6. Invalid argument to --signal and --ignorte not diagnosed:
$ ./exec --signal 12345; echo $?
0
$ ./exec --ignore 12345; echo $?
0
With the patch applied:
$ ./exec -t
./exec: Missing argument for -t
$ ./exec -t -1
./exec: Bad argument for -t: -1
$ ./exec -t 1x
./exec: Bad argument for -t: 1x
$ ./exec --exit
./exec: Missing argument for --exit
$ ./exec --exit=9999999999
./exec: Bad argument for --exit: 9999999999
$ ./exec --signal 12345
./exec: kill(20842, SIG#12345) failed: Invalid argument
$ ./exec --ignore 12345
./exec: sigaction(SIG#12345, ...) failed: Invalid argument
Index: /build/sebor/stdcxx/util/cmdopt.cpp
===================================================================
--- /build/sebor/stdcxx/util/cmdopt.cpp (revision 428458)
+++ /build/sebor/stdcxx/util/cmdopt.cpp (working copy)
@@ -157,10 +157,16 @@
int
eval_options (const int argc, char* const argv [])
{
+ const char opt_timeout[] = "-t";
+ const char opt_compat[] = "--compat";
+ const char opt_exit[] = "--exit";
+ const char opt_ignore[] = "--ignore";
+ const char opt_nocompat[] = "--nocompat";
+ const char opt_signal[] = "--signal";
+ const char opt_sleep[] = "--sleep";
+
int i;
- char* val;
-
assert (0 != argv);
if (1 == argc || '-' != argv [1][0])
@@ -168,6 +174,14 @@
for (i = 1; i < argc && '-' == argv [i][0]; ++i) {
+ /* the name of the option being processed */
+ const char* optname = argv [i];
+
+ /* the option's argument, if any */
+ const char* optarg = 0;
+
+ char* end = 0;
+
switch (argv [i][1]) {
case '?':
case 'h':
@@ -176,11 +190,18 @@
++i; /* Ignore -r option (makefile compat) */
break;
case 't':
- val = get_short_val (argv, &i);
- timeout = strtol (val, &val, 10);
- if (*val || errno)
- terminate (1, "Unknown value for -t: %s\n", val);
+ optname = opt_timeout;
+ optarg = get_short_val (argv, &i);
+ if (optarg) {
+ timeout = strtol (optarg, &end, 10);
+ if (*end || timeout < 0 || errno)
+ terminate (1, "Bad argument for %s: %s\n", optname,
optarg);
+ }
+ else
+ terminate (1, "Missing argument for %s\n", optname);
+
break;
+
case 'd':
in_root = get_short_val (argv, &i);
break;
@@ -193,6 +214,7 @@
case 'q':
verbose = 0;
break;
+
case '-':
{
const size_t arglen = strlen (argv [i]);
@@ -201,80 +223,83 @@
if ('\0' == argv [i][2])
return i+1;
- if (8 == arglen && 0 == memcmp ("--compat\0", argv [i], 9)) {
- compat = 1;
+ if ( sizeof opt_compat - 1 == arglen
+ && !memcmp (opt_compat, argv [i], sizeof opt_compat)) {
+ optname = opt_compat;
+ compat = 1;
break;
}
- else if (10 == arglen && 0 == memcmp ("--nocompat\0", argv [i],
- 11)) {
- compat = 0;
+ else if ( sizeof opt_nocompat - 1 == arglen
+ && !memcmp (opt_nocompat, argv [i], sizeof opt_nocompat))
{
+ optname = opt_nocompat;
+ compat = 0;
break;
}
- else if (6 <= arglen && 0 == memcmp ("--exit", argv [i], 6)) {
- val = get_long_val (argv, &i, 6);
- if (val) {
- int code = strtol (val, &val, 10);
- if (*val || errno)
- terminate (1, "Unknown value for --exit: %s\n", val);
- exit (code);
+ else if ( sizeof opt_exit - 1 <= arglen
+ && !memcmp (opt_exit, argv [i], sizeof opt_exit - 1)) {
+ optname = opt_exit;
+ optarg = get_long_val (argv, &i, 6);
+ if (optarg && *optarg) {
+ const long code = strtol (optarg, &end, 10);
+ if ('\0' == *end && !errno)
+ exit (code);
}
- else
- bad_option (argv [i]);
}
- else if (7 <= arglen && 0 == memcmp ("--sleep", argv [i], 7)) {
- val = get_long_val (argv, &i, 7);
- if (val) {
- int duration = strtol (val, &val, 10);
- if (*val || errno)
- terminate (1, "Unknown value for --sleep: %s\n", val);
- sleep (duration);
- break;
+ else if ( sizeof opt_sleep - 1 <= arglen
+ && !memcmp (opt_sleep, argv [i], sizeof opt_sleep - 1)) {
+ optname = opt_sleep;
+ optarg = get_long_val (argv, &i, 7);
+ if (optarg && *optarg) {
+ const long nsec = strtol (optarg, &end, 10);
+ if ('\0' == *end && 0 <= nsec && !errno) {
+ sleep (nsec);
+ break;
+ }
}
- else
- bad_option (argv [i]);
}
- else if (8 <= arglen && 0 == memcmp ("--signal", argv [i], 8)) {
- val = get_long_val (argv, &i, 8);
- if (val) {
- int sig = get_signo (val);
- if (0 > sig)
- terminate (1, "Unknown signal name for --signal: "
- "%s\n", val);
-
- /* Ignore kill errors (what should we do with them?) */
- (void)kill (getpid (), sig);
-
- /* Not certain what we should do if we don't terminate by
- signal */
- break;
+ else if ( sizeof opt_signal - 1 <= arglen
+ && !memcmp (opt_signal, argv [i], sizeof opt_signal - 1))
{
+ optname = opt_signal;
+ optarg = get_long_val (argv, &i, 8);
+ if (optarg && *optarg) {
+ const long signo = get_signo (optarg);
+ if (0 <= signo) {
+ if (0 > kill (getpid (), signo))
+ terminate (1, "kill(%d, %s) failed: %s\n",
+ getpid (), get_signame (signo),
+ strerror (errno));
+ break;
+ }
}
- else
- bad_option (argv [i]);
}
- else if (8 <= arglen && 0 == memcmp ("--ignore", argv [i], 8)) {
- val = get_long_val (argv, &i, 8);
- if (val) {
+ else if ( sizeof opt_ignore <= arglen
+ && !memcmp (opt_ignore, argv [i], sizeof opt_ignore - 1))
{
+ optname = opt_ignore;
+ optarg = get_long_val (argv, &i, 8);
+ if (optarg && *optarg) {
struct sigaction act;
- int sig = get_signo (val);
- if (0 > sig)
- terminate (1, "Unknown signal name for --ignore: "
- "%s\n", val);
-
- memset (&act, 0, sizeof act);
- act.sa_handler = SIG_IGN;
- (void)sigaction (sig, &act, 0);
- /* Ignore sigaction errors (what should we do with them?)
- */
- break;
+ const long signo = get_signo (optarg);
+ if (0 <= signo) {
+ memset (&act, 0, sizeof act);
+ act.sa_handler = SIG_IGN;
+ if (0 > sigaction (signo, &act, 0))
+ terminate (1, "sigaction(%s, ...) failed: %s\n",
+ get_signame (signo), strerror (errno));
+ break;
+ }
}
- else
- bad_option (argv [i]);
}
- else
- bad_option (argv [i]);
+
+ /* fall through */
}
default:
- bad_option (argv [i]);
+ if (optarg)
+ terminate (1, "Bad argument for %s: %s\n", optname, optarg);
+
+ if (argv [i])
+ bad_option (argv [i]);
+ else
+ terminate (1, "Missing argument for %s\n", optname);
}
}
