* small problem with kill --queue
@ 2014-07-07 20:34 Benno Schulenberg
2014-07-07 22:39 ` Sami Kerola
0 siblings, 1 reply; 6+ messages in thread
From: Benno Schulenberg @ 2014-07-07 20:34 UTC (permalink / raw)
To: Util-Linux; +Cc: Sami Kerola
Hello Sami,
./kill --queue 11 12
kill: sending signal to 12 failed: Operation not permitted
/kill --queue data 12
kill: unknown signal data; valid signals:
1 HUP 2 INT 3 QUIT 4 ILL 5 TRAP [...]
But "data" is not a signal, it is meant to be a number, an integer,
a value passed to the signalled process.
In misc-utils.c it does this:
arg = *argv;
if ((ctl->numsig = arg_to_signum(arg, 0)) < 0)
err_nosig(arg);
But it should not run arg_to_signum() on the argument, it
should simply parse it as an integer, and when that fails,
it should not call err_nosig() but say something like "not
a number". Right?
Benno
--
http://www.fastmail.fm - mmm... Fastmail...
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: small problem with kill --queue 2014-07-07 20:34 small problem with kill --queue Benno Schulenberg @ 2014-07-07 22:39 ` Sami Kerola 2014-07-09 13:29 ` Benno Schulenberg 0 siblings, 1 reply; 6+ messages in thread From: Sami Kerola @ 2014-07-07 22:39 UTC (permalink / raw) To: Benno Schulenberg; +Cc: Util-Linux On 7 July 2014 21:34, Benno Schulenberg <bensberg@justemail.net> wrote: > ./kill --queue 11 12 > kill: sending signal to 12 failed: Operation not permitted > > /kill --queue data 12 > kill: unknown signal data; valid signals: > 1 HUP 2 INT 3 QUIT 4 ILL 5 TRAP [...] > > But "data" is not a signal, it is meant to be a number, an integer, > a value passed to the signalled process. > > In misc-utils.c it does this: > > arg = *argv; > if ((ctl->numsig = arg_to_signum(arg, 0)) < 0) > err_nosig(arg); > > But it should not run arg_to_signum() on the argument, it > should simply parse it as an integer, and when that fails, > it should not call err_nosig() but say something like "not > a number". Right? Hi Benno, Right. I am not sure did I mess up or someone else earlier when --queue was introduced. Which way ever that was now when I look sigqueue(3) manual page your change proposal makes sense. Please have a look of the change below, that I hope to get reviewed before the final v2.25 is out. --->8---- From: Sami Kerola <kerolasa@iki.fi> Date: Mon, 7 Jul 2014 23:31:56 +0100 Subject: [PATCH] kill: use --queue argument as sigval integer The sigqueue(3) takes two values, signal and sigval. Contents of the signal can be altered with --signal option argument, so the --queue argument should be reserved to affect sigval_int. Reference: http://man7.org/linux/man-pages/man3/sigqueue.3.html Reported-by: Benno Schulenberg <bensberg@justemail.net> Signed-off-by: Sami Kerola <kerolasa@iki.fi> --- misc-utils/kill.1 | 18 ++++++++++++------ misc-utils/kill.c | 4 +--- tests/ts/kill/options | 4 ++-- 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/misc-utils/kill.1 b/misc-utils/kill.1 index dd1b1ee..65d094f 100644 --- a/misc-utils/kill.1 +++ b/misc-utils/kill.1 @@ -98,14 +98,20 @@ This functionality is deprecated, and will not be removed in March 2016. Use .BR sigqueue (2) rather than -.BR kill (2) -and the +.BR kill (2). +Argument .I sigval -argument is used to specify an integer to be sent with the signal. If the -receiving process has installed a handler for this signal using the SA_SIGINFO -flag to +is an integer that sent with the signal. If the receiving process has +installed a handler for this signal using the SA_SIGINFO flag to .BR sigaction (2), -then it can obtain this data via the si_value field of the siginfo_t structure. +then it can obtain the sigval integer via the si_value field of the +siginfo_t structure passed as the second argument to the handler. +The +.B \-\-queue +should be combined with +.BI \-\-signal " signal" +when other than a SIGTERM is wanted to accompanied with +.IR sigval . .SH NOTES It is not possible to send a signal to explicitly selected thread in a multithreaded process by diff --git a/misc-utils/kill.c b/misc-utils/kill.c index 5d2f27d..a24087f 100644 --- a/misc-utils/kill.c +++ b/misc-utils/kill.c @@ -412,9 +412,7 @@ static char **parse_arguments(int argc, char **argv, struct kill_control *ctl) errx(EXIT_FAILURE, _("%s and %s are mutually exclusive"), "--pid", "--queue"); argc--, argv++; arg = *argv; - if ((ctl->numsig = arg_to_signum(arg, 0)) < 0) - err_nosig(arg); - ctl->sigdata.sival_int = ctl->numsig; + ctl->sigdata.sival_int = strtos32_or_err(arg, _("argument error")); ctl->use_sigval = 1; continue; } diff --git a/tests/ts/kill/options b/tests/ts/kill/options index 6cdaa24..5af78d9 100755 --- a/tests/ts/kill/options +++ b/tests/ts/kill/options @@ -51,8 +51,8 @@ try_option -s 1 try_option --signal 1 try_option --signal HUP try_option --signal SIGHUP -try_option -q HUP -try_option --queue HUP +try_option -s 1 -q 42 +try_option -s 1 --queue 42 try_option -1 try_option -HUP try_option -SIGHUP -- 2.0.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: small problem with kill --queue 2014-07-07 22:39 ` Sami Kerola @ 2014-07-09 13:29 ` Benno Schulenberg 2014-07-09 20:49 ` Sami Kerola 0 siblings, 1 reply; 6+ messages in thread From: Benno Schulenberg @ 2014-07-09 13:29 UTC (permalink / raw) To: kerolasa; +Cc: Util-Linux Hi Sami, On Tue, Jul 8, 2014, at 00:39, Sami Kerola wrote: > Right. I am not sure did I mess up or someone else earlier when > --queue was introduced. It looks like you made the mistaken change four months ago, in commit 9e8dffd5cd29f03029b1ac99eecb129532ca5c0f. > Which way ever that was now when I look > sigqueue(3) manual page your change proposal makes sense. Please have > a look of the change below, that I hope to get reviewed before the > final v2.25 is out. [...] > > +The > +.B \-\-queue > +should be combined with > +.BI \-\-signal " signal" > +when other than a SIGTERM is wanted to accompanied with > +.IR sigval . I don't think it's necessary to explain again that wanting to send any other signal than TERM requires the --signal option; this is already at the top of the page. Also, thinking again, this word "sigval" looks far too similar to "signal", it would be much clearer to use the plain word "value" instead. So my upcoming patches for the usage text and man page use that instead. > + ctl->sigdata.sival_int = strtos32_or_err(arg, _("argument error")); Maybe "argument of --queue must be an integer" instead? My patch to kill.c also improves the error message when the signal name or number cannot be interpreted. Currently: ./kill --sigmal HUP foobar kill: invalid sigval argument Huh? New: ./kill --sigmal HUP foobar kill: invalid signal name or number: -sigmal Ah, typo. Benno -- http://www.fastmail.fm - Access all of your messages and folders wherever you are ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: small problem with kill --queue 2014-07-09 13:29 ` Benno Schulenberg @ 2014-07-09 20:49 ` Sami Kerola 2014-07-09 20:49 ` [PATCH] kill: use --queue option argument as sigval integer value Sami Kerola 0 siblings, 1 reply; 6+ messages in thread From: Sami Kerola @ 2014-07-09 20:49 UTC (permalink / raw) To: util-linux; +Cc: kerolasa On 9 July 2014 14:29, Benno Schulenberg <bensberg@justemail.net> wrote: > On Tue, Jul 8, 2014, at 00:39, Sami Kerola wrote: >> Right. I am not sure did I mess up or someone else earlier when >> --queue was introduced. > > It looks like you made the mistaken change four months ago, > in commit 9e8dffd5cd29f03029b1ac99eecb129532ca5c0f. Yep. Another bug I created. Oops & sorry. >> Which way ever that was now when I look >> sigqueue(3) manual page your change proposal makes sense. Please have >> a look of the change below, that I hope to get reviewed before the >> final v2.25 is out. [...] >> >> +The >> +.B \-\-queue >> +should be combined with >> +.BI \-\-signal " signal" >> +when other than a SIGTERM is wanted to accompanied with >> +.IR sigval . > > I don't think it's necessary to explain again that wanting to send any > other signal than TERM requires the --signal option; this is already at > the top of the page. > > Also, thinking again, this word "sigval" looks far too similar to "signal", > it would be much clearer to use the plain word "value" instead. So my > upcoming patches for the usage text and man page use that instead. I tried to think possible person who is trying to write program(s) that would use the kill --queue mechanism. Maybe it is good enoug for such persons to find this email conversation from web archive, with remark the --queue <value> is the same value as in sigaction(3p) SA_SIGINFO void func(int signo, siginfo_t *info, void *context); the 'si_value' value in 'siginfo_t' struture, and in sigqueue(3) int sigqueue(pid_t pid, int sig, const union sigval value); the 'int sival_int' in 'union sigval'. That said, the following patch looks good to me. [PATCH 2/2] docs: fix many wording and some formatting issues in the man page of kill >> + ctl->sigdata.sival_int = strtos32_or_err(arg, _("argument error")); > > Maybe "argument of --queue must be an integer" instead? > > My patch to kill.c also improves the error message when the > signal name or number cannot be interpreted. Currently: > > ./kill --sigmal HUP foobar > kill: invalid sigval argument > > Huh? > > New: > > ./kill --sigmal HUP foobar > kill: invalid signal name or number: -sigmal You took care of this already. [PATCH 1/2] textual: fix the usage message of kill Incoming change from me[*] is intented to be applied on top of the textual fix. [*] [PATCH] kill: use --queue option argument as sigval integer value -- Sami Kerola http://www.iki.fi/kerolasa/ ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] kill: use --queue option argument as sigval integer value 2014-07-09 20:49 ` Sami Kerola @ 2014-07-09 20:49 ` Sami Kerola 2014-07-14 13:51 ` Karel Zak 0 siblings, 1 reply; 6+ messages in thread From: Sami Kerola @ 2014-07-09 20:49 UTC (permalink / raw) To: util-linux; +Cc: kerolasa The sigqueue(3) takes two values, signal and sigval. Contents of the signal can be altered with --signal option argument, so the --queue argument should be reserved to affect sigval_int. This is regression fix introduced by commit 9e8dffd5cd29f03029b1ac99eecb129532ca5c0f. Reference: http://man7.org/linux/man-pages/man3/sigqueue.3.html Reported-by: Benno Schulenberg <bensberg@justemail.net> Signed-off-by: Sami Kerola <kerolasa@iki.fi> --- misc-utils/kill.c | 4 +--- tests/ts/kill/options | 4 ++-- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/misc-utils/kill.c b/misc-utils/kill.c index f2362c3..2f89427 100644 --- a/misc-utils/kill.c +++ b/misc-utils/kill.c @@ -412,9 +412,7 @@ static char **parse_arguments(int argc, char **argv, struct kill_control *ctl) errx(EXIT_FAILURE, _("%s and %s are mutually exclusive"), "--pid", "--queue"); argc--, argv++; arg = *argv; - if ((ctl->numsig = arg_to_signum(arg, 0)) < 0) - err_nosig(arg); - ctl->sigdata.sival_int = ctl->numsig; + ctl->sigdata.sival_int = strtos32_or_err(arg, _("argument error")); ctl->use_sigval = 1; continue; } diff --git a/tests/ts/kill/options b/tests/ts/kill/options index 6cdaa24..5af78d9 100755 --- a/tests/ts/kill/options +++ b/tests/ts/kill/options @@ -51,8 +51,8 @@ try_option -s 1 try_option --signal 1 try_option --signal HUP try_option --signal SIGHUP -try_option -q HUP -try_option --queue HUP +try_option -s 1 -q 42 +try_option -s 1 --queue 42 try_option -1 try_option -HUP try_option -SIGHUP -- 2.0.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] kill: use --queue option argument as sigval integer value 2014-07-09 20:49 ` [PATCH] kill: use --queue option argument as sigval integer value Sami Kerola @ 2014-07-14 13:51 ` Karel Zak 0 siblings, 0 replies; 6+ messages in thread From: Karel Zak @ 2014-07-14 13:51 UTC (permalink / raw) To: Sami Kerola; +Cc: util-linux On Wed, Jul 09, 2014 at 09:49:03PM +0100, Sami Kerola wrote: > misc-utils/kill.c | 4 +--- > tests/ts/kill/options | 4 ++-- > 2 files changed, 3 insertions(+), 5 deletions(-) Applied, thanks. -- Karel Zak <kzak@redhat.com> http://karelzak.blogspot.com ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-07-14 13:51 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-07-07 20:34 small problem with kill --queue Benno Schulenberg 2014-07-07 22:39 ` Sami Kerola 2014-07-09 13:29 ` Benno Schulenberg 2014-07-09 20:49 ` Sami Kerola 2014-07-09 20:49 ` [PATCH] kill: use --queue option argument as sigval integer value Sami Kerola 2014-07-14 13:51 ` Karel Zak
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.