skill: fix command line with signal, again

Have skill_sig_option sanitize the command line by properly decrementing
*argc after moving the arguments to remove the -signal one.

One bug caused by this issue was when running `kill -1`, then the code
would interpret -1 as both SIGHUP and as process group -1 and send
SIGHUP to all of them. Or `kill -28` which would send SIGWINCH to
process group -2 (in another bug, the -pgid support only accepts a
single digit, fix for that bug will follow.)

This also reverts commit 7610b3128e6ac4 ("skill: fix command line with
signal") which worked around this bug in `skill` and also removes the
"sigopt" hack which worked around this bug in `kill`.

The skill_sig_option implementation is compatible with signal_option()
from pgrep.c. I plan to factor them out into a single source file in a
follow up commit, to prevent the duplication.

This commit fixes the issues reported above. I also tested the issues
from commit 7610b3128e6ac4, `skill -9 -t pts/0` works as expected, also
tried `kill` with -signal and a number of pids and it worked as
expected.

Also tested that `make check` and `make distcheck` keep working.

Signed-off-by: Filipe Brandenburger <filbranden@google.com>

diff --git a/skill.c b/skill.c
index 9a19a8bbd8a64eb5a0af5a370f75c4b889f6ee71..9e499739fea735b99c4dca7695577f2f9d84d18c 100644 (file)
--- a/skill.c
+++ b/skill.c
@@ -398,17 +398,15 @@ static void __attribute__ ((__noreturn__)) skillsnice_usage(FILE * out)
 
 static int skill_sig_option(int *argc, char **argv)
 {
-       int i, nargs = *argc;
+       int i;
        int signo = -1;
-       for (i = 1; i < nargs; i++) {
+       for (i = 1; i < *argc; i++) {
                if (argv[i][0] == '-') {
                        signo = signal_name_to_number(argv[i] + 1);
                        if (-1 < signo) {
-                               if (nargs - i) {
-                                       nargs--;
-                                       memmove(argv + i, argv + i + 1,
-                                               sizeof(char *) * (nargs - i));
-                               }
+                               memmove(argv + i, argv + i + 1,
+                                       sizeof(char *) * (*argc - i));
+                               (*argc)--;
                                return signo;
                        }
                }
@@ -421,7 +419,6 @@ static void __attribute__ ((__noreturn__))
     kill_main(int argc, char **argv)
 {
        int signo, i;
-       int sigopt = 0;
        int loop = 1;
        long pid;
        int exitvalue = EXIT_SUCCESS;
@@ -446,8 +443,6 @@ static void __attribute__ ((__noreturn__))
        signo = skill_sig_option(&argc, argv);
        if (signo < 0)
                signo = SIGTERM;
-       else
-               sigopt++;
 
        opterr=0; /* suppress errors on -123 */
        while (loop == 1 && (i = getopt_long(argc, argv, "l::Ls:hV", longopts, NULL)) != -1)
@@ -495,7 +490,7 @@ static void __attribute__ ((__noreturn__))
                        kill_usage(stderr);
                }
 
-       argc -= optind + sigopt;
+       argc -= optind;
        argv += optind;
 
        for (i = 0; i < argc; i++) {
@@ -588,10 +583,8 @@ static void skillsnice_parse(int argc,
                prino = snice_prio_option(&argc, argv);
        else if (program == PROG_SKILL) {
                signo = skill_sig_option(&argc, argv);
-               if (-1 < signo) {
+               if (-1 < signo)
                        sig_or_pri = signo;
-                       argc -= 1;
-               }
        }
 
        pid_count = 0;