ash: fix ash-signals/signal8 testcase failure
authorDenys Vlasenko <vda.linux@googlemail.com>
Mon, 21 Feb 2011 02:22:20 +0000 (03:22 +0100)
committerDenys Vlasenko <vda.linux@googlemail.com>
Mon, 21 Feb 2011 02:22:20 +0000 (03:22 +0100)
function                                             old     new   delta
killcmd                                              109     224    +115
kill_main                                            882     910     +28
changepath                                           194     195      +1
------------------------------------------------------------------------------
(add/remove: 0/0 grow/shrink: 3/0 up/down: 144/0)             Total: 144 bytes

Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>
procps/kill.c
shell/ash.c
shell/ash_test/ash-signals/sigint1.right [new file with mode: 0644]
shell/ash_test/ash-signals/sigint1.tests [new file with mode: 0755]
shell/hush_test/hush-misc/sigint1.right [new file with mode: 0644]
shell/hush_test/hush-misc/sigint1.tests [new file with mode: 0755]

index b51d44a70a1837b7dd51779f356f3e804a387b7f..39538016ecd0ba1836b98789e66278a3cd928cea 100644 (file)
@@ -206,9 +206,27 @@ int kill_main(int argc, char **argv)
 
        /* Looks like they want to do a kill. Do that */
        while (arg) {
-               /* Support shell 'space' trick */
-               if (arg[0] == ' ')
-                       arg++;
+#if ENABLE_ASH || ENABLE_HUSH
+               /*
+                * We need to support shell's "hack formats" of
+                * " -PRGP_ID" (yes, with a leading space)
+                * and " PID1 PID2 PID3" (with degenerate case "")
+                */
+               while (*arg != '\0') {
+                       char *end;
+                       if (*arg == ' ')
+                               arg++;
+                       pid = bb_strtoi(arg, &end, 10);
+                       if (errno && (errno != EINVAL || *end != ' ')) {
+                               bb_error_msg("invalid number '%s'", arg);
+                               errors++;
+                       } else if (kill(pid, signo) != 0) {
+                               bb_perror_msg("can't kill pid %d", (int)pid);
+                               errors++;
+                       }
+                       arg = end; /* can only point to ' ' or '\0' now */
+               }
+#else
                pid = bb_strtoi(arg, NULL, 10);
                if (errno) {
                        bb_error_msg("invalid number '%s'", arg);
@@ -217,6 +235,7 @@ int kill_main(int argc, char **argv)
                        bb_perror_msg("can't kill pid %d", (int)pid);
                        errors++;
                }
+#endif
                arg = *++argv;
        }
        return errors;
index cccd6dd79393b749284fa9596dc59dcc0e592efa..98d2c7c29edb47728ee653e86027b39c1a1f03a9 100644 (file)
@@ -3783,18 +3783,51 @@ setjobctl(int on)
 static int FAST_FUNC
 killcmd(int argc, char **argv)
 {
-       int i = 1;
        if (argv[1] && strcmp(argv[1], "-l") != 0) {
+               int i = 1;
                do {
                        if (argv[i][0] == '%') {
-                               struct job *jp = getjob(argv[i], 0);
-                               unsigned pid = jp->ps[0].ps_pid;
-                               /* Enough space for ' -NNN<nul>' */
-                               argv[i] = alloca(sizeof(int)*3 + 3);
-                               /* kill_main has matching code to expect
-                                * leading space. Needed to not confuse
-                                * negative pids with "kill -SIGNAL_NO" syntax */
-                               sprintf(argv[i], " -%u", pid);
+                               /*
+                                * "kill %N" - job kill
+                                * Converting to pgrp / pid kill
+                                */
+                               struct job *jp;
+                               char *dst;
+                               int j, n;
+
+                               jp = getjob(argv[i], 0);
+                               /*
+                                * In jobs started under job control, we signal
+                                * entire process group by kill -PGRP_ID.
+                                * This happens, f.e., in interactive shell.
+                                *
+                                * Otherwise, we signal each child via
+                                * kill PID1 PID2 PID3.
+                                * Testcases:
+                                * sh -c 'sleep 1|sleep 1 & kill %1'
+                                * sh -c 'true|sleep 2 & sleep 1; kill %1'
+                                * sh -c 'true|sleep 1 & sleep 2; kill %1'
+                                */
+                               n = jp->nprocs; /* can't be 0 (I hope) */
+                               if (jp->jobctl)
+                                       n = 1;
+                               dst = alloca(n * sizeof(int)*4);
+                               argv[i] = dst;
+                               for (j = 0; j < n; j++) {
+                                       struct procstat *ps = &jp->ps[j];
+                                       /* Skip non-running and not-stopped members
+                                        * (i.e. dead members) of the job
+                                        */
+                                       if (ps->ps_status != -1 && !WIFSTOPPED(ps->ps_status))
+                                               continue;
+                                       /*
+                                        * kill_main has matching code to expect
+                                        * leading space. Needed to not confuse
+                                        * negative pids with "kill -SIGNAL_NO" syntax
+                                        */
+                                       dst += sprintf(dst, jp->jobctl ? " -%u" : " %u", (int)ps->ps_pid);
+                               }
+                               *dst = '\0';
                        }
                } while (argv[++i]);
        }
@@ -4227,8 +4260,9 @@ waitcmd(int argc UNUSED_PARAM, char **argv)
                                        break;
                                job = job->prev_job;
                        }
-               } else
+               } else {
                        job = getjob(*argv, 0);
+               }
                /* loop until process terminated or stopped */
                while (job->state == JOBRUNNING)
                        blocking_wait_with_raise_on_sig();
@@ -4724,7 +4758,7 @@ forkchild(struct job *jp, union node *n, int mode)
 #if JOBS
        /* do job control only in root shell */
        doing_jobctl = 0;
-       if (mode != FORK_NOJOB && jp->jobctl && !oldlvl) {
+       if (mode != FORK_NOJOB && jp->jobctl && oldlvl == 0) {
                pid_t pgrp;
 
                if (jp->nprocs == 0)
@@ -4750,7 +4784,7 @@ forkchild(struct job *jp, union node *n, int mode)
                                ash_msg_and_raise_error("can't open '%s'", bb_dev_null);
                }
        }
-       if (!oldlvl) {
+       if (oldlvl == 0) {
                if (iflag) { /* why if iflag only? */
                        setsignal(SIGINT);
                        setsignal(SIGTERM);
diff --git a/shell/ash_test/ash-signals/sigint1.right b/shell/ash_test/ash-signals/sigint1.right
new file mode 100644 (file)
index 0000000..a9094b0
--- /dev/null
@@ -0,0 +1 @@
+Sending SIGINT to main shell PID
diff --git a/shell/ash_test/ash-signals/sigint1.tests b/shell/ash_test/ash-signals/sigint1.tests
new file mode 100755 (executable)
index 0000000..20e45d9
--- /dev/null
@@ -0,0 +1,41 @@
+# What should happen if non-interactive shell gets SIGINT?
+
+(sleep 1; echo Sending SIGINT to main shell PID; exec kill -INT $$) &
+
+# We create a child which exits with 0 even on SIGINT
+# (This is truly necessary only if SIGINT is generated by ^C,
+# in this testcase even bare "sleep 2" would do because
+# we don't send SIGINT _to_ the_ child_...)
+$THIS_SH -c 'trap "exit 0" SIGINT; sleep 2'
+
+# In one second, we (main shell) get SIGINT here.
+# The question is whether we should, or should not, exit.
+
+# bash will not stop here. It will execute next command(s).
+
+# The rationale for this is described here:
+# http://www.cons.org/cracauer/sigint.html
+#
+# Basically, bash will not exit on SIGINT immediately if it waits
+# for a child. It will wait for the child to exit.
+# If child exits NOT by dying on SIGINT, then bash will not exit.
+#
+# The idea is that the following script:
+# | emacs file.txt
+# | more cmds
+# User may use ^C to interrupt editor's ops like search. But then
+# emacs exits normally. User expects that script doesn't stop.
+#
+# This is a nice idea, but detecting "did process really exit
+# with SIGINT?" is racy. Consider:
+# | bash -c 'while true; do /bin/true; done'
+# When ^C is pressed while bash waits for /bin/true to exit,
+# it may happen that /bin/true exits with exitcode 0 before
+# ^C is delivered to it as SIGINT. bash will see SIGINT, then
+# it will see that child exited with 0, and bash will NOT EXIT.
+
+# Therefore we do not implement bash behavior.
+# I'd say that emacs need to put itself into a separate pgrp
+# to isolate shell from getting stray SIGINTs from ^C.
+
+echo Next command after SIGINT was executed
diff --git a/shell/hush_test/hush-misc/sigint1.right b/shell/hush_test/hush-misc/sigint1.right
new file mode 100644 (file)
index 0000000..a9094b0
--- /dev/null
@@ -0,0 +1 @@
+Sending SIGINT to main shell PID
diff --git a/shell/hush_test/hush-misc/sigint1.tests b/shell/hush_test/hush-misc/sigint1.tests
new file mode 100755 (executable)
index 0000000..20e45d9
--- /dev/null
@@ -0,0 +1,41 @@
+# What should happen if non-interactive shell gets SIGINT?
+
+(sleep 1; echo Sending SIGINT to main shell PID; exec kill -INT $$) &
+
+# We create a child which exits with 0 even on SIGINT
+# (This is truly necessary only if SIGINT is generated by ^C,
+# in this testcase even bare "sleep 2" would do because
+# we don't send SIGINT _to_ the_ child_...)
+$THIS_SH -c 'trap "exit 0" SIGINT; sleep 2'
+
+# In one second, we (main shell) get SIGINT here.
+# The question is whether we should, or should not, exit.
+
+# bash will not stop here. It will execute next command(s).
+
+# The rationale for this is described here:
+# http://www.cons.org/cracauer/sigint.html
+#
+# Basically, bash will not exit on SIGINT immediately if it waits
+# for a child. It will wait for the child to exit.
+# If child exits NOT by dying on SIGINT, then bash will not exit.
+#
+# The idea is that the following script:
+# | emacs file.txt
+# | more cmds
+# User may use ^C to interrupt editor's ops like search. But then
+# emacs exits normally. User expects that script doesn't stop.
+#
+# This is a nice idea, but detecting "did process really exit
+# with SIGINT?" is racy. Consider:
+# | bash -c 'while true; do /bin/true; done'
+# When ^C is pressed while bash waits for /bin/true to exit,
+# it may happen that /bin/true exits with exitcode 0 before
+# ^C is delivered to it as SIGINT. bash will see SIGINT, then
+# it will see that child exited with 0, and bash will NOT EXIT.
+
+# Therefore we do not implement bash behavior.
+# I'd say that emacs need to put itself into a separate pgrp
+# to isolate shell from getting stray SIGINTs from ^C.
+
+echo Next command after SIGINT was executed