Merge branch 'sigoption1' into 'master'

Fixes to option parsing in kill, skill, pkill.

Hi,

These are some fixes to how kill, skill and pkill handle parameters in the -SIGNUM form.

The handling was incorrect in kill/skill, it was actually not properly truncating argc/argv before removing the -SIGNUM argument. There were some hacks around the code to work around the bug, but using `kill -1` on its own would end up working as if `kill -1 -1` (which means `kill -HUP <all processes>`) was executed. (Yes, it was painful when I accidentally typed it... more than once.)

I also made `kill` print the usage if it only gets a signal number, but no pids (as in the `kill -1` example.)

And `pkill` had similar code but was using a buggy atoi() to try to parse the signal number, which meant that trailing garbage was accepted, so I corrected that by removing the buggy atoi() and letting the already existing code that already accepted numeric signals do its work as supposed to...

I also wanted to tackle the "pgrp" case where a negative number is passed to kill, causing it to kill a process group. The current code is buggy, for instance `kill -TERM -2345` will kill process group 2 and not process group 2345 as supposed to. It should also be possible to pass it multiple pgrps or a mix of pgrps and pids. It's hard to fix that though, considering how getopt_long() works, so I'll defer that for a second pull request.

I tested this fairly well, both manually and made sure there were no regressions in the test suite, also didn't break `make distcheck`.

Let me know if you have any questions or other remarks...

Cheers!
Filipe

See merge request !3
This commit is contained in:
Craig Small 2015-07-08 12:08:08 +00:00
commit 9c2bb7fed4
2 changed files with 10 additions and 16 deletions

View File

@ -661,8 +661,6 @@ static int signal_option(int *argc, char **argv)
for (i = 1; i < *argc; i++) { for (i = 1; i < *argc; i++) {
if (argv[i][0] == '-') { if (argv[i][0] == '-') {
sig = signal_name_to_number(argv[i] + 1); sig = signal_name_to_number(argv[i] + 1);
if (sig == -1 && isdigit(argv[i][1]))
sig = atoi(argv[i] + 1);
if (-1 < sig) { if (-1 < sig) {
memmove(argv + i, argv + i + 1, memmove(argv + i, argv + i + 1,
sizeof(char *) * (*argc - i)); sizeof(char *) * (*argc - i));

24
skill.c
View File

@ -398,17 +398,15 @@ static void __attribute__ ((__noreturn__)) skillsnice_usage(FILE * out)
static int skill_sig_option(int *argc, char **argv) static int skill_sig_option(int *argc, char **argv)
{ {
int i, nargs = *argc; int i;
int signo = -1; int signo = -1;
for (i = 1; i < nargs; i++) { for (i = 1; i < *argc; i++) {
if (argv[i][0] == '-') { if (argv[i][0] == '-') {
signo = signal_name_to_number(argv[i] + 1); signo = signal_name_to_number(argv[i] + 1);
if (-1 < signo) { if (-1 < signo) {
if (nargs - i) { memmove(argv + i, argv + i + 1,
nargs--; sizeof(char *) * (*argc - i));
memmove(argv + i, argv + i + 1, (*argc)--;
sizeof(char *) * (nargs - i));
}
return signo; return signo;
} }
} }
@ -421,7 +419,6 @@ static void __attribute__ ((__noreturn__))
kill_main(int argc, char **argv) kill_main(int argc, char **argv)
{ {
int signo, i; int signo, i;
int sigopt = 0;
int loop = 1; int loop = 1;
long pid; long pid;
int exitvalue = EXIT_SUCCESS; int exitvalue = EXIT_SUCCESS;
@ -446,8 +443,6 @@ static void __attribute__ ((__noreturn__))
signo = skill_sig_option(&argc, argv); signo = skill_sig_option(&argc, argv);
if (signo < 0) if (signo < 0)
signo = SIGTERM; signo = SIGTERM;
else
sigopt++;
opterr=0; /* suppress errors on -123 */ opterr=0; /* suppress errors on -123 */
while (loop == 1 && (i = getopt_long(argc, argv, "l::Ls:hV", longopts, NULL)) != -1) while (loop == 1 && (i = getopt_long(argc, argv, "l::Ls:hV", longopts, NULL)) != -1)
@ -495,9 +490,12 @@ static void __attribute__ ((__noreturn__))
kill_usage(stderr); kill_usage(stderr);
} }
argc -= optind + sigopt; argc -= optind;
argv += optind; argv += optind;
if (argc < 1)
kill_usage(stderr);
for (i = 0; i < argc; i++) { for (i = 0; i < argc; i++) {
pid = strtol_or_err(argv[i], _("failed to parse argument")); pid = strtol_or_err(argv[i], _("failed to parse argument"));
if (!kill((pid_t) pid, signo)) if (!kill((pid_t) pid, signo))
@ -588,10 +586,8 @@ static void skillsnice_parse(int argc,
prino = snice_prio_option(&argc, argv); prino = snice_prio_option(&argc, argv);
else if (program == PROG_SKILL) { else if (program == PROG_SKILL) {
signo = skill_sig_option(&argc, argv); signo = skill_sig_option(&argc, argv);
if (-1 < signo) { if (-1 < signo)
sig_or_pri = signo; sig_or_pri = signo;
argc -= 1;
}
} }
pid_count = 0; pid_count = 0;