Date: Thu, 19 Jun 2008 07:03:10 GMT From: Garrett Cooper <gcooper@FreeBSD.org> To: freebsd-gnats-submit@FreeBSD.org Subject: misc/124748: sh -c 'exit -1' fails with "Illegal number: -1", instead of exiting with a code of 255 Message-ID: <200806190703.m5J73Af5033644@www.freebsd.org> Resent-Message-ID: <200806190710.m5J7A1k4070053@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>Number: 124748
>Category: misc
>Synopsis: sh -c 'exit -1' fails with "Illegal number: -1", instead of exiting with a code of 255
>Confidential: no
>Severity: non-critical
>Priority: medium
>Responsible: freebsd-bugs
>State: open
>Quarter:
>Keywords:
>Date-Required:
>Class: sw-bug
>Submitter-Id: current-users
>Arrival-Date: Thu Jun 19 07:10:01 UTC 2008
>Closed-Date:
>Last-Modified:
>Originator: Garrett Cooper
>Release: 8-CURRENT
>Organization:
n/a
>Environment:
FreeBSD optimus 8.0-CURRENT FreeBSD 8.0-CURRENT #6: Mon Jun 9 20:41:28 PDT 2008 root@optimus:/usr/obj/usr/src/sys/OPTIMUS i386
>Description:
Bourne shell (/bin/sh), as it stands, doesn't parse numbers properly, even though bash does:
[gcooper@optimus /devel/ncvs/src/bin/sh]$ sh -c 'exit -1'
exit: Illegal number: -1
[gcooper@optimus /devel/ncvs/src/bin/sh]$ sh -c 'exit +1'
exit: Illegal number: +1
These are valid numbers though, which are merely red herrings; the patch attached does fix this behavior though, according to the following gamut of tests:
[gcooper@optimus /devel/ncvs/src/bin/sh]$ cat number_test.sh
#!/bin/sh
for shell in sh $@; do
for i in -1 1 +1; do
echo "$shell -c \"exit $i\""
$shell -c "exit $i"; echo $?
done;
done
# End of test
[gcooper@optimus /devel/ncvs/src/bin/sh]$ ./number_test.sh bash ./sh
sh -c "exit -1"
exit: Illegal number: -1
2
sh -c "exit 1"
1
sh -c "exit +1"
exit: Illegal number: +1
2
bash -c "exit -1"
255
bash -c "exit 1"
1
bash -c "exit +1"
1
./sh -c "exit -1"
255
./sh -c "exit 1"
1
./sh -c "exit +1"
1
>How-To-Repeat:
/bin/sh -c "exit -1"
/bin/sh -c "exit +1"
>Fix:
Patch attached (also fixes histcmd.c and trap.c -- strange thing is that I can't seem to test that builtin cmd), and maintains proper compatibility with builtin jobs:
[gcooper@optimus /devel/ncvs/src/bin/sh]$ sh -c 'builtin histcmd'
histcmd: not found
Patch attached with submission follows:
Index: exec.c
===================================================================
RCS file: /home/ncvs/src/bin/sh/exec.c,v
retrieving revision 1.31
diff -r1.31 exec.c
40a41
> #include <string.h>
361c362
< if (prefix("builtin", pathopt)) {
---
> if (strncmp(BUILTIN, pathopt, strlen(BUILTIN)) == 0) {
Index: histedit.c
===================================================================
RCS file: /home/ncvs/src/bin/sh/histedit.c,v
retrieving revision 1.29
diff -r1.29 histedit.c
425,426d424
< if (*s == '-')
< s++;
Index: jobs.c
===================================================================
RCS file: /home/ncvs/src/bin/sh/jobs.c,v
retrieving revision 1.72
diff -r1.72 jobs.c
534c534
< return (jp);
---
> return jp;
540c540,544
< jobno = number(name + 1);
---
> /*
> * Don't use number() here. This breaks existing
> * jobs(1) compatibility.
> */
> jobno = atoi(name + 1);
560c564
< if (found)
---
> if (found != NULL)
571,572c575,576
< && prefix(name + 1, jp->ps[0].cmd)) {
< if (found)
---
> && strncmp(name+1, jp->ps[0].cmd, strlen(name+1))) {
> if (found != NULL)
577,578c581,582
< if (found)
< return found;
---
> if (found != NULL)
> return (found);
581c585,586
< pid = (pid_t)number(name);
---
> /* no need to run is_number() again with number() */
> pid = (pid_t)atoi(name);
Index: main.h
===================================================================
RCS file: /home/ncvs/src/bin/sh/main.h,v
retrieving revision 1.8
diff -r1.8 main.h
38a39,40
> #define BUILTIN "builtin"
>
Index: mystring.c
===================================================================
RCS file: /home/ncvs/src/bin/sh/mystring.c,v
retrieving revision 1.13
diff -r1.13 mystring.c
56a57,59
> #if DEBUG
> #include <stdio.h>
> #endif
123a127,135
> /* Is this the first index? */
> int iter = 0;
>
> #if DEBUG
> #define PRINT_DEBUG fprintf(stderr, "%d: %c\n", iter, *p)
> #else
> #define PRINT_DEBUG
> #endif
>
125c137,154
< if (! is_digit(*p))
---
> /*
> * Account for signs in front of numbers.
> */
>
> /*
> * XXX: does POSIX bourne shell allow for '+' prefixed
> * numbers?
> */
>
> /*
> * The string defined by *p isn't a number, unless:
> * 1. It's a digit.
> * 2. The 0'th index is either a + or -.
> */
> if (!(is_digit(*p) ||
> (iter == 0 && (*p == '-' || *p == '+')))
> ) {
> PRINT_DEBUG;
126a156,163
> }
>
> PRINT_DEBUG;
>
> #undef PRINT_DEBUG
>
> iter++;
>
>Release-Note:
>Audit-Trail:
>Unformatted:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200806190703.m5J73Af5033644>