[src/trunk]: src/bin/sh Fix several problems with the implementation of the "...

[kre <kre%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/e6c7624e2faa
branches:  trunk
changeset: 823627:e6c7624e2faa
user:      kre <kre%NetBSD.org@localhost>
date:      Sat Apr 29 15:12:21 2017 +0000

description:
Fix several problems with the implementation of the "trap" command
(that is, with the command itself, not with the traps that are
executed, if any).

- "trap -- -l" is not rational, permit the (non-std) -l option only
  when given as the sole arg (ie: "trap -l").
- "trap --" is the same as just "trap" (and -- is ignored for below)
- "trap action" generates a usage message (there must be at least one condition)
- "trap N [condition...]" (the old form with a numeric first arg, to reset
  traps to default, instead of "trap - condition...") is properly detected.
  In particular while "trap 1 2 3" resets sighup sigint and siquit handlers
  to default, "trap hup int quit" runs the "hup" command on sigint or sigquit
  and does nothing to sighup at all.
- actions can start with "-" (as can commands in general) - it may be unusual
  or even unwise, but it is not prohibited, and should work
- bad conditions (signal names/numbers) are just a usage error (resulting in
  non-zero "exit status" (and a diagnostic on stderr)) they do not cause
  the script to abort (as a syntax error in a special builtin would.)
  (so says posix, very explicitly.)
- when outputting the trap list ("trap") properly quote null actions
  (ignored conditions).  This has the side effect of also generating an
  explicit null string ('') in other cases where null values are output,
  such as when reporting var values ("set") but that's OK, and might be
  better (VAR= and VAR='' mean the same, but the latter is more obvious.)

We still do not properly handle traps=$(trap) (ie: it does not work at all,
and should) but that's a different problem that needs fixing in another place.

diffstat:

 bin/sh/trap.c |  61 +++++++++++++++++++++++++++++++++-------------------------
 bin/sh/var.c  |   8 +++++-
 2 files changed, 41 insertions(+), 28 deletions(-)

diffs (133 lines):

diff -r 19356defeb4a -r e6c7624e2faa bin/sh/trap.c
--- a/bin/sh/trap.c     Sat Apr 29 14:43:09 2017 +0000
+++ b/bin/sh/trap.c     Sat Apr 29 15:12:21 2017 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: trap.c,v 1.38 2017/04/26 22:41:53 kre Exp $    */
+/*     $NetBSD: trap.c,v 1.39 2017/04/29 15:12:21 kre Exp $    */
 
 /*-
  * Copyright (c) 1991, 1993
@@ -37,7 +37,7 @@
 #if 0
 static char sccsid[] = "@(#)trap.c     8.5 (Berkeley) 6/5/95";
 #else
-__RCSID("$NetBSD: trap.c,v 1.38 2017/04/26 22:41:53 kre Exp $");
+__RCSID("$NetBSD: trap.c,v 1.39 2017/04/29 15:12:21 kre Exp $");
 #endif
 #endif /* not lint */
 
@@ -136,6 +136,19 @@
        char *action;
        char **ap;
        int signo;
+       int errs = 0;
+
+       ap = argv + 1;
+
+       if (argc == 2 && strcmp(*ap, "-l") == 0) {
+               printsignals();
+               return 0;
+       }
+
+       if (argc > 1 && strcmp(*ap, "--") == 0) {
+               argc--;
+               ap++;
+       }
 
        if (argc <= 1) {
                for (signo = 0 ; signo <= NSIG ; signo++)
@@ -147,37 +160,33 @@
                        }
                return 0;
        }
-       ap = argv + 1;
 
        action = NULL;
 
-       if (strcmp(*ap, "--") == 0)
-               if (*++ap == NULL)
-                       return 0;
-
-       if (signame_to_signum(*ap) == -1) {
-               if ((*ap)[0] == '-') {
-                       if ((*ap)[1] == '\0')
-                               ap++;
-                       else if ((*ap)[1] == 'l' && (*ap)[2] == '\0') {
-                               printsignals();
-                               return 0;
-                       }
-                       else
-                               error("bad option %s\n", *ap);
-               }
+       if (!is_number(*ap)) {
+               if ((*ap)[0] == '-' && (*ap)[1] == '\0')
+                       ap++;                   /* reset to default */
                else
-                       action = *ap++;
+                       action = *ap++;         /* can be '' for "ignore" */
+               argc--;
        }
 
+       if (argc < 2) {         /* there must be at least 1 condition */
+               out2str("Usage: trap [-l]\n"
+                       "       trap action condition ...\n"
+                       "       trap N condition ...\n");
+               return 2;
+       }
+
+
        while (*ap) {
-               if (is_number(*ap))
-                       signo = number(*ap);
-               else
-                       signo = signame_to_signum(*ap);
+               signo = signame_to_signum(*ap);
 
-               if (signo < 0 || signo > NSIG)
-                       error("%s: bad trap", *ap);
+               if (signo < 0 || signo > NSIG) {
+                       /* This is not a fatal error, so sayeth posix */
+                       outfmt(out2, "trap: '%s' bad condition\n", *ap);
+                       errs = 1;
+               }
 
                INTOFF;
                if (action)
@@ -193,7 +202,7 @@
                INTON;
                ap++;
        }
-       return 0;
+       return errs;
 }
 
 
diff -r 19356defeb4a -r e6c7624e2faa bin/sh/var.c
--- a/bin/sh/var.c      Sat Apr 29 14:43:09 2017 +0000
+++ b/bin/sh/var.c      Sat Apr 29 15:12:21 2017 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: var.c,v 1.49 2016/03/31 16:16:35 christos Exp $        */
+/*     $NetBSD: var.c,v 1.50 2017/04/29 15:12:21 kre Exp $     */
 
 /*-
  * Copyright (c) 1991, 1993
@@ -37,7 +37,7 @@
 #if 0
 static char sccsid[] = "@(#)var.c      8.3 (Berkeley) 5/4/95";
 #else
-__RCSID("$NetBSD: var.c,v 1.49 2016/03/31 16:16:35 christos Exp $");
+__RCSID("$NetBSD: var.c,v 1.50 2017/04/29 15:12:21 kre Exp $");
 #endif
 #endif /* not lint */
 
@@ -497,6 +497,10 @@
 {
        const char *q;
 
+       if (p[0] == '\0') {
+               out1fmt("''");
+               return;
+       }
        if (strcspn(p, "|&;<>()$`\\\"' \t\n*?[]#~=%") == strlen(p)) {
                out1fmt("%s", p);
                return;