Module Name:    src
Committed By:   kre
Date:           Mon Jun 26 22:09:16 UTC 2017

Modified Files:
        src/bin/kill: kill.c
        src/bin/sh: var.h
        src/bin/sh/bltin: bltin.h

Log Message:
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.)


To generate a diff of this commit:
cvs rdiff -u -r1.27 -r1.28 src/bin/kill/kill.c
cvs rdiff -u -r1.31 -r1.32 src/bin/sh/var.h
cvs rdiff -u -r1.14 -r1.15 src/bin/sh/bltin/bltin.h

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/bin/kill/kill.c
diff -u src/bin/kill/kill.c:1.27 src/bin/kill/kill.c:1.28
--- src/bin/kill/kill.c:1.27	Mon Aug 29 14:51:18 2011
+++ src/bin/kill/kill.c	Mon Jun 26 22:09:16 2017
@@ -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 @@ __COPYRIGHT("@(#) Copyright (c) 1988, 19
 #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 @@ int killcmd(int, char *argv[]);
 #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,65 +85,92 @@ main(int argc, char *argv[])
 	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 (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]);
+			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);
-		}
-		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;
-		argc--, argv++;
-	} else if (**argv == '-') {
-		char *sn = *argv + 1;
-		if (isalpha((unsigned char)*sn)) {
-			if ((numsig = signame_to_signum(sn)) < 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 (strcmp(sn, "0") == 0)
+				numsig = 0;
+			else if ((numsig = signalnumber(sn)) == 0) {
+				if (sn != argv[0])
+					goto trysignal;
 				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)
+			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);
-		} else
-			nosig(sn);
-		argc--, argv++;
+			argc--, argv++;
+			break;
+		}
 	}
 
+	/* deal with the optional '--' end of options option */
+	if (argc > 0 && strcmp(*argv, "--") == 0)
+		argc--, argv++;
+
 	if (argc == 0)
 		usage();
 
@@ -157,26 +186,23 @@ main(int argc, char *argv[])
 			}
 		} 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 @@ main(int argc, char *argv[])
 }
 
 static int
-signame_to_signum(char *sig)
+signum(const char *sn)
+{
+	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;
+}
+
+static pid_t
+processnum(const char *s)
 {
-	int n;
+	intmax_t n;
+	char *ep;
+
+	n = strtoimax(s, &ep, 10);
 
-	if (strncasecmp(sig, "sig", 3) == 0)
-		sig += 3;
-	for (n = 1; n < NSIG; n++) {
-		if (!strcasecmp(sys_signame[n], sig))
-			return (n);
+	/* 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 @@ static void
 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;
+
+		nl = strlen(name);
 
-		if (len + nl >= termwidth) {
+		if (len > 0 && nl + len + pad >= termwidth) {
 			fprintf(fp, "\n");
 			len = 0;
-		} else
-			if (len != 0)
-				fprintf(fp, " ");
-		len += nl;
+			pad = 0;
+		} else if (pad > 0 && len != 0)
+			fprintf(fp, "%*s", pad, "");
+		else
+			pad = 0;
+
+		len += nl + pad;
+		pad = (nl | 7) + 1 - nl;
+
 		fprintf(fp, "%s", name);
 	}
 	if (len != 0)
@@ -242,12 +299,13 @@ printsignals(FILE *fp)
 static void
 usage(void)
 {
+	const char *pn = getprogname();
 
 	fprintf(stderr, "usage: %s [-s signal_name] pid ...\n"
-	    "       %s -l [exit_status]\n"
-	    "       %s -signal_name pid ...\n"
-	    "       %s -signal_number pid ...\n",
-	    getprogname(), getprogname(), getprogname(), getprogname());
+			"       %s -l [exit_status]\n"
+			"       %s -signal_name pid ...\n"
+			"       %s -signal_number pid ...\n",
+	    pn, pn, pn, pn);
 	exit(1);
 	/* NOTREACHED */
 }

Index: src/bin/sh/var.h
diff -u src/bin/sh/var.h:1.31 src/bin/sh/var.h:1.32
--- src/bin/sh/var.h:1.31	Sat Jun 17 10:46:34 2017
+++ src/bin/sh/var.h	Mon Jun 26 22:09:16 2017
@@ -1,4 +1,4 @@
-/*	$NetBSD: var.h,v 1.31 2017/06/17 10:46:34 kre Exp $	*/
+/*	$NetBSD: var.h,v 1.32 2017/06/26 22:09:16 kre Exp $	*/
 
 /*-
  * Copyright (c) 1991, 1993
@@ -34,6 +34,7 @@
  *	@(#)var.h	8.2 (Berkeley) 5/4/95
  */
 
+#ifndef	VUNSET		/* double include protection */
 /*
  * Shell variables.
  */
@@ -137,3 +138,5 @@ int unsetvar(const char *, int);
 void choose_ps1(void);
 int setvarsafe(const char *, const char *, int);
 void print_quoted(const char *);
+
+#endif

Index: src/bin/sh/bltin/bltin.h
diff -u src/bin/sh/bltin/bltin.h:1.14 src/bin/sh/bltin/bltin.h:1.15
--- src/bin/sh/bltin/bltin.h:1.14	Wed Mar 16 22:36:21 2016
+++ src/bin/sh/bltin/bltin.h	Mon Jun 26 22:09:16 2017
@@ -1,4 +1,4 @@
-/*	$NetBSD: bltin.h,v 1.14 2016/03/16 22:36:21 christos Exp $	*/
+/*	$NetBSD: bltin.h,v 1.15 2017/06/26 22:09:16 kre Exp $	*/
 
 /*-
  * Copyright (c) 1991, 1993
@@ -51,6 +51,7 @@
 #ifdef SHELL
 #include "../output.h"
 #include "../error.h"
+#include "../var.h"
 #undef stdout
 #undef stderr
 #undef putc

Reply via email to