Andrew Black wrote:
This looks good but a few minor changes are needed before I can
commit it -- please see my comments below.
[...]
+const int
+get_signo (char* signame)
The argument should be const char*...
+{
...and we should assert (0 != signame) here since that's what the
the function assumes.
size_t i;
+
+ if ('S' == signame [0] && 'I' == signame [1] && 'G' == signame [2])
+ signame += 3;
It would be nice to also handle lowercase letters like the kill
command does (this is not a showstopper but it seems easy enough
to implement that I would suggest to include it with the rest of
the changes).
+
+ if ('#' == signame [0])
+ ++signame;
+
+ if ('0' <= signame [0] && '9' >= signame [0])
+ return atoi (signame);
We should use strtol here and detect invalid input such as "SIG1XXX".
+
+ for (i = 0; i < name_count; ++i) {
+ if (0 == strcmp (signal_names [i].str, signame)) {
+ return signal_names [i].val;
Lowercase handling also needs to be done here.
[...]
Index: exec.h
===================================================================
--- exec.h (revision 427578)
+++ exec.h (working copy)
@@ -32,6 +32,8 @@
int killed;
};
+const int get_signo (char* const signame);
Let's use const char* consistently.
[...]
+static char*
+get_short_val (char* const* argv, int* idx)
+{
Assert all preconditions please.
[...]
+static char*
+get_long_val (char* const * argv, int* idx, unsigned offset)
+{
Assert preconditions.
[...]
+static void
+bad_option (char* const opt)
+{
Assert preconditions.
+ printf ("Unknown option: %s\n", opt);
This should (continue to) go to stderr.
Also, shouldn't this be handled via a call to terminate() so that
the output is consistent and includes the name of the executable?
[...]
@@ -132,19 +191,70 @@
[...]
+ else if (6 <= arglen && 0 == memcmp ("--exit", argv [i], 6)) {
+ val = get_long_val (argv, &i, 6);
+ if (val)
+ exit (atoi (val));
I would also suggest using strtol() here (and probably everywhere
else we're using atoi) and handling malformed input for robustness.
Thanks
Martin
