Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/bin Make arg parsing in kill POSIX compatible with POSIX (XB...



details:   https://anonhg.NetBSD.org/src/rev/90bdd2710e85
branches:  trunk
changeset: 825022:90bdd2710e85
user:      kre <kre%NetBSD.org@localhost>
date:      Mon Jun 26 22:09:16 2017 +0000

description:
Make arg parsing in kill POSIX compatible with POSIX (XBD 2.12) by
parsing the way getopt(3) would, if only it could handle the (required)
-signumber and -signame options.  This adds two "features" to kill,
-ssigname and -lstatus now work (ie: one word with all of the '-', the
option letter, and its value) and "--" also now works (kill -- -pid1 pid2
will not attempt to send the pid1 signal to pid2, but rather SIGTERM
to the pid1 process group and pid2).  It is still the case that (apart
from --) at most 1 option is permitted (-l, -s, -signame, or -signumber.)

Note that we now have an ambiguity, -sname might mean "-s name" or
send the signal "sname" - if one of those turns out to be valid, that
will be accepted, otherwise the error message will indicate that "sname"
is not a valid signal name, not that "name" is not.   Keeping the "-s"
and signal name as separate words avoids this issue.

Also caution: should someone be weird enough to define a new signal
name (as in the part after SIG) which is almost the same name as an
existing name that starts with 'S' by adding an extra 'S' prepended
(eg: adding a SIGSSYS) then the ambiguity problem becomes much worse.
In that case "kill -ssys" will be resolved in favour of the "-s"
flag being used (the more modern syntax) and would send a SIGSYS, rather
that a SIGSSYS.    So don't do that.

While here, switch to using signalname(3) (bye bye NSIG, et. al.), add
some constipation, and show a little pride in formatting the signal names
for "kill -l" (and in the usage when appropriate -- same routine.)   Respect
COLUMNS (POSIX XBD 8.3) as primary specification of the width (terminal width,
not number of columns to print) for kill -l, a very small value for COLUMNS
will cause kill -l output to list signals one per line, a very large
value will cause them all to be listed on one line.) (eg: "COLUMNS=1 kill -l")

TODO: the signal printing for "trap -l" and that for "kill -l"
should be switched to use a common routine (for the sh builtin versions.)

All changes of relevance here are to bin/kill - the (minor) changes to bin/sh
are only to properly expose the builtin version of getenv(3) so the builtin
version of kill can use it (ie: make its prototype available.)

diffstat:

 bin/kill/kill.c      |  244 +++++++++++++++++++++++++++++++-------------------
 bin/sh/bltin/bltin.h |    3 +-
 bin/sh/var.h         |    5 +-
 3 files changed, 157 insertions(+), 95 deletions(-)

diffs (truncated from 380 to 300 lines):

diff -r 9bd28a511d06 -r 90bdd2710e85 bin/kill/kill.c
--- a/bin/kill/kill.c   Mon Jun 26 20:36:01 2017 +0000
+++ b/bin/kill/kill.c   Mon Jun 26 22:09:16 2017 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: kill.c,v 1.27 2011/08/29 14:51:18 joerg Exp $ */
+/* $NetBSD: kill.c,v 1.28 2017/06/26 22:09:16 kre Exp $ */
 
 /*
  * Copyright (c) 1988, 1993, 1994
@@ -39,7 +39,7 @@
 #if 0
 static char sccsid[] = "@(#)kill.c     8.4 (Berkeley) 4/28/95";
 #else
-__RCSID("$NetBSD: kill.c,v 1.27 2011/08/29 14:51:18 joerg Exp $");
+__RCSID("$NetBSD: kill.c,v 1.28 2017/06/26 22:09:16 kre Exp $");
 #endif
 #endif /* not lint */
 
@@ -63,17 +63,19 @@
 #include "../../bin/sh/bltin/bltin.h"
 #endif /* SHELL */ 
 
-__dead static void nosig(char *);
+__dead static void nosig(const char *);
 static void printsignals(FILE *);
-static int signame_to_signum(char *);
+static int signum(const char *);
+static pid_t processnum(const char *);
 __dead static void usage(void);
 
 int
 main(int argc, char *argv[])
 {
        int errors;
-       intmax_t numsig, pid;
-       char *ep;
+       int numsig;
+       pid_t pid;
+       const char *sn;
 
        setprogname(argv[0]);
        setlocale(LC_ALL, "");
@@ -83,64 +85,91 @@
        numsig = SIGTERM;
 
        argc--, argv++;
-       if (strcmp(*argv, "-l") == 0) {
-               argc--, argv++;
-               if (argc > 1)
-                       usage();
-               if (argc == 1) {
-                       if (isdigit((unsigned char)**argv) == 0)
+
+       /*
+        * Process exactly 1 option, if there is one.
+        */
+       if (argv[0][0] == '-') {
+               switch (argv[0][1]) {
+               case 'l':
+                       if (argv[0][2] != '\0')
+                               sn = argv[0] + 2;
+                       else {
+                               argc--; argv++;
+                               sn = argv[0];
+                       }
+                       if (argc > 1)
                                usage();
-                       numsig = strtoimax(*argv, &ep, 10);
-                       /* check for correctly parsed number */
-                       if (*ep != '\0' || numsig == INTMAX_MIN || numsig == INTMAX_MAX) {
-                               errx(EXIT_FAILURE, "illegal signal number: %s",
-                                               *argv);
-                               /* NOTREACHED */
+                       if (argc == 1) {
+                               if (isdigit((unsigned char)*sn) == 0)
+                                       usage();
+                               numsig = signum(sn);
+                               if (numsig >= 128)
+                                       numsig -= 128;
+                               if (numsig == 0 || signalnext(numsig) == -1)
+                                       nosig(sn);
+                               sn = signalname(numsig);
+                               if (sn == NULL)
+                                       errx(EXIT_FAILURE,
+                                          "unknown signal number: %d", numsig);
+                               printf("%s\n", sn);
+                               exit(0);
+                       }
+                       printsignals(stdout);
+                       exit(0);
+
+               case 's':
+                       if (argv[0][2] != '\0')
+                               sn = argv[0] + 2;
+                       else {
+                               argc--, argv++;
+                               if (argc < 1) {
+                                       warnx(
+                                           "option requires an argument -- s");
+                                       usage();
+                               }
+                               sn = argv[0];
                        }
-                       if (numsig >= 128)
-                               numsig -= 128;
-                       /* and whether it fits into signals range */
-                       if (numsig <= 0 || numsig >= NSIG)
-                               nosig(*argv);
-                       printf("%s\n", sys_signame[(int) numsig]);
-                       exit(0);
+                       if (strcmp(sn, "0") == 0)
+                               numsig = 0;
+                       else if ((numsig = signalnumber(sn)) == 0) {
+                               if (sn != argv[0])
+                                       goto trysignal;
+                               nosig(sn);
+                       }
+                       argc--, argv++;
+                       break;
+
+               case '-':
+                       if (argv[0][2] == '\0') {
+                               /* process this one again later */
+                               break;
+                       }
+                       /* FALL THROUGH */
+               case '\0':
+                       usage();
+                       break;
+
+               default:
+ trysignal:
+                       sn = *argv + 1;
+                       if (((numsig = signalnumber(sn)) == 0)) {
+                               if (isdigit((unsigned char)*sn))
+                                       numsig = signum(sn);
+                               else
+                                       nosig(sn);
+                       }
+
+                       if (numsig != 0 && signalnext(numsig) == -1)
+                               nosig(sn);
+                       argc--, argv++;
+                       break;
                }
-               printsignals(stdout);
-               exit(0);
        }
 
-       if (!strcmp(*argv, "-s")) {
-               argc--, argv++;
-               if (argc < 1) {
-                       warnx("option requires an argument -- s");
-                       usage();
-               }
-               if (strcmp(*argv, "0")) {
-                       if ((numsig = signame_to_signum(*argv)) < 0)
-                               nosig(*argv);
-               } else
-                       numsig = 0;
+       /* deal with the optional '--' end of options option */
+       if (argc > 0 && strcmp(*argv, "--") == 0)
                argc--, argv++;
-       } else if (**argv == '-') {
-               char *sn = *argv + 1;
-               if (isalpha((unsigned char)*sn)) {
-                       if ((numsig = signame_to_signum(sn)) < 0)
-                               nosig(sn);
-               } else if (isdigit((unsigned char)*sn)) {
-                       numsig = strtoimax(sn, &ep, 10);
-                       /* check for correctly parsed number */
-                       if (*ep || numsig == INTMAX_MIN || numsig == INTMAX_MAX ) {
-                               errx(EXIT_FAILURE, "illegal signal number: %s",
-                                               sn);
-                               /* NOTREACHED */
-                       }
-                       /* and whether it fits into signals range */
-                       if (numsig < 0 || numsig >= NSIG)
-                               nosig(sn);
-               } else
-                       nosig(sn);
-               argc--, argv++;
-       }
 
        if (argc == 0)
                usage();
@@ -157,26 +186,23 @@
                        }
                } else 
 #endif
-               {
-                       pid = strtoimax(*argv, &ep, 10);
-                       /* make sure the pid is a number and fits into pid_t */
-                       if (!**argv || *ep || pid == INTMAX_MIN ||
-                               pid == INTMAX_MAX || pid != (pid_t) pid) {
-
-                               warnx("illegal process id: %s", *argv);
+                       if ((pid = processnum(*argv)) == (pid_t)-1) {
                                errors = 1;
                                continue;
                        }
-               }
-               if (kill((pid_t) pid, (int) numsig) == -1) {
+
+               if (kill(pid, numsig) == -1) {
                        warn("%s", *argv);
                        errors = 1;
                }
 #ifdef SHELL
-               /* Wakeup the process if it was suspended, so it can
-                  exit without an explicit 'fg'. */
+               /*
+                * Wakeup the process if it was suspended, so it can
+                * exit without an explicit 'fg'.
+                *      (kernel handles this for SIGKILL)
+                */
                if (numsig == SIGTERM || numsig == SIGHUP)
-                       kill((pid_t) pid, SIGCONT);
+                       kill(pid, SIGCONT);
 #endif
        }
 
@@ -185,21 +211,41 @@
 }
 
 static int
-signame_to_signum(char *sig)
+signum(const char *sn)
 {
-       int n;
+       intmax_t n;
+       char *ep;
+
+       n = strtoimax(sn, &ep, 10);
+
+       /* check for correctly parsed number */
+       if (*ep || n <= INT_MIN || n >= INT_MAX )
+               errx(EXIT_FAILURE, "illegal signal number: %s", sn);
+               /* NOTREACHED */
+
+       return (int)n;
+}
 
-       if (strncasecmp(sig, "sig", 3) == 0)
-               sig += 3;
-       for (n = 1; n < NSIG; n++) {
-               if (!strcasecmp(sys_signame[n], sig))
-                       return (n);
+static pid_t
+processnum(const char *s)
+{
+       intmax_t n;
+       char *ep;
+
+       n = strtoimax(s, &ep, 10);
+
+       /* check for correctly parsed number */
+       if (*ep || n == INTMAX_MIN || n == INTMAX_MAX || (pid_t)n != n ||
+           n == -1) {
+               warnx("illegal process%s id: %s", (n < 0 ? " group" : ""), s);
+               n = -1;
        }
-       return (-1);
+
+       return (pid_t)n;
 }
 
 static void
-nosig(char *name)
+nosig(const char *name)
 {
 
        warnx("unknown signal %s; valid signals:", name);
@@ -212,27 +258,38 @@
 printsignals(FILE *fp)
 {
        int sig;
-       int len, nl;
+       int len, nl, pad;
        const char *name;
        int termwidth = 80;
 
-       if (isatty(fileno(fp))) {
+       if ((name = getenv("COLUMNS")) != 0)
+               termwidth = atoi(name);
+       else if (isatty(fileno(fp))) {
                struct winsize win;
+
                if (ioctl(fileno(fp), TIOCGWINSZ, &win) == 0 && win.ws_col > 0)
                        termwidth = win.ws_col;
        }
 
-       for (len = 0, sig = 1; sig < NSIG; sig++) {
-               name = sys_signame[sig];
-               nl = 1 + strlen(name);
+       for (pad = 0, len = 0, sig = 0; (sig = signalnext(sig)) != 0; ) {
+               name = signalname(sig);
+               if (name == NULL)
+                       continue;
 



Home | Main Index | Thread Index | Old Index