* [PATCH 02/10] kill: fix coding style
2014-03-09 20:30 [PATCH 01/10] findfs: use symbolic exit values, and tell about them in manual Sami Kerola
@ 2014-03-09 20:30 ` Sami Kerola
2014-04-07 9:36 ` Karel Zak
2014-03-09 20:30 ` [PATCH 03/10] kill: flip all comparions to be in smaller - greater order Sami Kerola
` (10 subsequent siblings)
11 siblings, 1 reply; 24+ messages in thread
From: Sami Kerola @ 2014-03-09 20:30 UTC (permalink / raw)
To: util-linux; +Cc: kerolasa
The kill was deprecated at the time lot of other tools got style
unification. Now when deprecation is lifted it is time to get kill
cleaner. This commit does not modify code, only various spacing issues,
removal of unecessary braces, and such are dealt.
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
misc-utils/kill.c | 536 ++++++++++++++++++++++++++----------------------------
1 file changed, 259 insertions(+), 277 deletions(-)
diff --git a/misc-utils/kill.c b/misc-utils/kill.c
index 63fc2ac..e7bf5f4 100644
--- a/misc-utils/kill.c
+++ b/misc-utils/kill.c
@@ -42,16 +42,16 @@
*
*/
+#include <ctype.h> /* for isdigit() */
+#include <signal.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
-#include <ctype.h> /* for isdigit() */
#include <unistd.h>
-#include <signal.h>
#include "c.h"
-#include "nls.h"
#include "closestream.h"
+#include "nls.h"
#include "procutils.h"
#include "strutils.h"
#include "ttyutils.h"
@@ -63,38 +63,38 @@ struct signv {
} sys_signame[] = {
/* POSIX signals */
{ "HUP", SIGHUP }, /* 1 */
- { "INT", SIGINT }, /* 2 */
- { "QUIT", SIGQUIT }, /* 3 */
- { "ILL", SIGILL }, /* 4 */
+ { "INT", SIGINT }, /* 2 */
+ { "QUIT", SIGQUIT }, /* 3 */
+ { "ILL", SIGILL }, /* 4 */
#ifdef SIGTRAP
{ "TRAP", SIGTRAP }, /* 5 */
#endif
- { "ABRT", SIGABRT }, /* 6 */
+ { "ABRT", SIGABRT }, /* 6 */
#ifdef SIGIOT
- { "IOT", SIGIOT }, /* 6, same as SIGABRT */
+ { "IOT", SIGIOT }, /* 6, same as SIGABRT */
#endif
#ifdef SIGEMT
- { "EMT", SIGEMT }, /* 7 (mips,alpha,sparc*) */
+ { "EMT", SIGEMT }, /* 7 (mips,alpha,sparc*) */
#endif
#ifdef SIGBUS
{ "BUS", SIGBUS }, /* 7 (arm,i386,m68k,ppc), 10 (mips,alpha,sparc*) */
#endif
- { "FPE", SIGFPE }, /* 8 */
- { "KILL", SIGKILL }, /* 9 */
- { "USR1", SIGUSR1 }, /* 10 (arm,i386,m68k,ppc), 30 (alpha,sparc*), 16 (mips) */
- { "SEGV", SIGSEGV }, /* 11 */
- { "USR2", SIGUSR2 }, /* 12 (arm,i386,m68k,ppc), 31 (alpha,sparc*), 17 (mips) */
- { "PIPE", SIGPIPE }, /* 13 */
- { "ALRM", SIGALRM }, /* 14 */
- { "TERM", SIGTERM }, /* 15 */
+ { "FPE", SIGFPE }, /* 8 */
+ { "KILL", SIGKILL }, /* 9 */
+ { "USR1", SIGUSR1 }, /* 10 (arm,i386,m68k,ppc), 30 (alpha,sparc*), 16 (mips) */
+ { "SEGV", SIGSEGV }, /* 11 */
+ { "USR2", SIGUSR2 }, /* 12 (arm,i386,m68k,ppc), 31 (alpha,sparc*), 17 (mips) */
+ { "PIPE", SIGPIPE }, /* 13 */
+ { "ALRM", SIGALRM }, /* 14 */
+ { "TERM", SIGTERM }, /* 15 */
#ifdef SIGSTKFLT
{ "STKFLT", SIGSTKFLT }, /* 16 (arm,i386,m68k,ppc) */
#endif
- { "CHLD", SIGCHLD }, /* 17 (arm,i386,m68k,ppc), 20 (alpha,sparc*), 18 (mips) */
+ { "CHLD", SIGCHLD }, /* 17 (arm,i386,m68k,ppc), 20 (alpha,sparc*), 18 (mips) */
#ifdef SIGCLD
{ "CLD", SIGCLD }, /* same as SIGCHLD (mips) */
#endif
- { "CONT", SIGCONT }, /* 18 (arm,i386,m68k,ppc), 19 (alpha,sparc*), 25 (mips) */
+ { "CONT", SIGCONT }, /* 18 (arm,i386,m68k,ppc), 19 (alpha,sparc*), 25 (mips) */
{ "STOP", SIGSTOP }, /* 19 (arm,i386,m68k,ppc), 17 (alpha,sparc*), 23 (mips) */
{ "TSTP", SIGTSTP }, /* 20 (arm,i386,m68k,ppc), 18 (alpha,sparc*), 24 (mips) */
{ "TTIN", SIGTTIN }, /* 21 (arm,i386,m68k,ppc,alpha,sparc*), 26 (mips) */
@@ -127,7 +127,7 @@ struct signv {
{ "INFO", SIGINFO }, /* 29 (alpha) */
#endif
#ifdef SIGLOST
- { "LOST", SIGLOST }, /* 29 (arm,i386,m68k,ppc,sparc*) */
+ { "LOST", SIGLOST }, /* 29 (arm,i386,m68k,ppc,sparc*) */
#endif
#ifdef SIGPWR
{ "PWR", SIGPWR }, /* 30 (arm,i386,m68k,ppc), 29 (alpha,sparc*), 19 (mips) */
@@ -136,179 +136,171 @@ struct signv {
{ "UNUSED", SIGUNUSED }, /* 31 (arm,i386,m68k,ppc) */
#endif
#ifdef SIGSYS
- { "SYS", SIGSYS }, /* 31 (mips,alpha,sparc*) */
+ { "SYS", SIGSYS }, /* 31 (mips,alpha,sparc*) */
#endif
};
-static int arg_to_signum (char *arg, int mask);
-static void nosig (char *name);
-static void printsig (int sig);
-static void printsignals (FILE *fp, int pretty);
-static int usage (int status);
-static int kill_verbose (char *procname, int pid, int sig);
+static int arg_to_signum(char *arg, int mask);
+static void nosig(char *name);
+static void printsig(int sig);
+static void printsignals(FILE *fp, int pretty);
+static int usage(int status);
+static int kill_verbose(char *procname, int pid, int sig);
#ifdef HAVE_SIGQUEUE
static int use_sigval;
static union sigval sigdata;
#endif
-int main (int argc, char *argv[])
+int main(int argc, char **argv)
{
- int errors, numsig, pid;
- char *ep, *arg;
- int do_pid, do_kill, check_all;
-
- setlocale(LC_ALL, "");
- bindtextdomain(PACKAGE, LOCALEDIR);
- textdomain(PACKAGE);
- atexit(close_stdout);
-
- numsig = SIGTERM;
- do_pid = (! strcmp (program_invocation_short_name, "pid")); /* Yecch */
- do_kill = 0;
- check_all = 0;
-
- /* loop through the arguments.
- actually, -a is the only option can be used with other options.
- `kill' is basically a one-option-at-most program. */
- for (argc--, argv++; argc > 0; argc--, argv++) {
- arg = *argv;
- if (*arg != '-') {
- break;
- }
- if (! strcmp (arg, "--")) {
- argc--, argv++;
- break;
- }
- if (! strcmp (arg, "-v") || ! strcmp (arg, "-V") ||
- ! strcmp (arg, "--version")) {
- printf(UTIL_LINUX_VERSION);
- return EXIT_SUCCESS;
- }
- if (! strcmp (arg, "-h") || ! strcmp (arg, "--help"))
- return usage(EXIT_FAILURE);
-
- if (! strcmp (arg, "-a") || ! strcmp (arg, "--all")) {
- check_all++;
- continue;
- }
- if (! strcmp (arg, "-l") || ! strcmp (arg, "--list")) {
- if (argc < 2) {
- printsignals (stdout, 0);
- return EXIT_SUCCESS;
- }
- if (argc > 2)
- return usage (EXIT_FAILURE);
- /* argc == 2, accept "kill -l $?" */
- arg = argv[1];
- if ((numsig = arg_to_signum (arg, 1)) < 0)
- errx(EXIT_FAILURE, _("unknown signal: %s"), arg);
- printsig (numsig);
- return EXIT_SUCCESS;
- }
- /* for compatibility with procps kill(1) */
- if (! strncmp (arg, "--list=", 7) || ! strncmp (arg, "-l=", 3)) {
- char *p = strchr(arg, '=') + 1;
- if ((numsig = arg_to_signum(p, 1)) < 0)
- errx(EXIT_FAILURE, _("unknown signal: %s"), p);
- printsig (numsig);
- return EXIT_SUCCESS;
- }
- if (! strcmp (arg, "-L") || ! strcmp (arg, "--table")) {
- printsignals (stdout, 1);
- return EXIT_SUCCESS;
- }
- if (! strcmp (arg, "-p") || ! strcmp (arg, "--pid")) {
- do_pid++;
- if (do_kill)
- return usage (EXIT_FAILURE);
- continue;
- }
- if (! strcmp (arg, "-s") || ! strcmp (arg, "--signal")) {
- if (argc < 2) {
- return usage (EXIT_FAILURE);
- }
- do_kill++;
- if (do_pid)
- return usage (EXIT_FAILURE);
- argc--, argv++;
- arg = *argv;
- if ((numsig = arg_to_signum (arg, 0)) < 0) {
- nosig (arg);
- return EXIT_FAILURE;
- }
- continue;
- }
- if (! strcmp (arg, "-q") || ! strcmp (arg, "--queue")) {
- if (argc < 2)
- return usage (EXIT_FAILURE);
- argc--, argv++;
- arg = *argv;
+ int errors, numsig, pid;
+ char *ep, *arg;
+ int do_pid, do_kill, check_all;
+
+ setlocale(LC_ALL, "");
+ bindtextdomain(PACKAGE, LOCALEDIR);
+ textdomain(PACKAGE);
+ atexit(close_stdout);
+
+ numsig = SIGTERM;
+ do_pid = (!strcmp(program_invocation_short_name, "pid")); /* Yecch */
+ do_kill = 0;
+ check_all = 0;
+ /* Loop through the arguments. Actually, -a is the only option
+ * can be used with other options. The 'kill' is basically a
+ * one-option-at-most program. */
+ for (argc--, argv++; argc > 0; argc--, argv++) {
+ arg = *argv;
+ if (*arg != '-')
+ break;
+ if (!strcmp(arg, "--")) {
+ argc--, argv++;
+ break;
+ }
+ if (!strcmp(arg, "-v") || !strcmp(arg, "-V") ||
+ !strcmp(arg, "--version")) {
+ printf(UTIL_LINUX_VERSION);
+ return EXIT_SUCCESS;
+ }
+ if (!strcmp(arg, "-h") || !strcmp(arg, "--help"))
+ return usage(EXIT_FAILURE);
+
+ if (!strcmp(arg, "-a") || !strcmp(arg, "--all")) {
+ check_all++;
+ continue;
+ }
+ if (!strcmp(arg, "-l") || !strcmp(arg, "--list")) {
+ if (argc < 2) {
+ printsignals(stdout, 0);
+ return EXIT_SUCCESS;
+ }
+ if (argc > 2)
+ return usage(EXIT_FAILURE);
+ /* argc == 2, accept "kill -l $?" */
+ arg = argv[1];
+ if ((numsig = arg_to_signum(arg, 1)) < 0)
+ errx(EXIT_FAILURE, _("unknown signal: %s"),
+ arg);
+ printsig(numsig);
+ return EXIT_SUCCESS;
+ }
+ /* for compatibility with procps kill(1) */
+ if (!strncmp(arg, "--list=", 7) || !strncmp(arg, "-l=", 3)) {
+ char *p = strchr(arg, '=') + 1;
+ if ((numsig = arg_to_signum(p, 1)) < 0)
+ errx(EXIT_FAILURE, _("unknown signal: %s"), p);
+ printsig(numsig);
+ return EXIT_SUCCESS;
+ }
+ if (!strcmp(arg, "-L") || !strcmp(arg, "--table")) {
+ printsignals(stdout, 1);
+ return EXIT_SUCCESS;
+ }
+ if (!strcmp(arg, "-p") || !strcmp(arg, "--pid")) {
+ do_pid++;
+ if (do_kill)
+ return usage(EXIT_FAILURE);
+ continue;
+ }
+ if (!strcmp(arg, "-s") || !strcmp(arg, "--signal")) {
+ if (argc < 2)
+ return usage(EXIT_FAILURE);
+ do_kill++;
+ if (do_pid)
+ return usage(EXIT_FAILURE);
+ argc--, argv++;
+ arg = *argv;
+ if ((numsig = arg_to_signum(arg, 0)) < 0) {
+ nosig(arg);
+ return EXIT_FAILURE;
+ }
+ continue;
+ }
+ if (!strcmp(arg, "-q") || !strcmp(arg, "--queue")) {
+ if (argc < 2)
+ return usage(EXIT_FAILURE);
+ argc--, argv++;
+ arg = *argv;
#ifdef HAVE_SIGQUEUE
- sigdata.sival_int = strtos32_or_err(arg, _("invalid sigval argument"));
- use_sigval = 1;
+ sigdata.sival_int =
+ strtos32_or_err(arg, _("invalid sigval argument"));
+ use_sigval = 1;
#endif
- continue;
- }
- /* `arg' begins with a dash but is not a known option.
- so it's probably something like -HUP, or -1/-n
- try to deal with it.
- -n could be signal n, or pid -n (i.e. process group n).
- In case of doubt POSIX tells us to assume a signal.
- If a signal has been parsed, assume it's a pid, break */
- if (do_kill)
- break;
- arg++;
- if ((numsig = arg_to_signum (arg, 0)) < 0) {
- return usage (EXIT_FAILURE);
+ continue;
+ }
+ /* 'arg' begins with a dash but is not a known option.
+ * So it's probably something like -HUP, or -1/-n try to
+ * deal with it.
+ *
+ * -n could be signal n, or pid -n (i.e., process group
+ * number). In case of doubt POSIX tells us to assume a
+ * signal. If a signal has been parsed, assume it is a
+ * pid, break. */
+ if (do_kill)
+ break;
+ arg++;
+ if ((numsig = arg_to_signum(arg, 0)) < 0)
+ return usage(EXIT_FAILURE);
+ do_kill++;
+ if (do_pid)
+ return usage(EXIT_FAILURE);
+ continue;
}
- do_kill++;
+ if (!*argv)
+ return usage(EXIT_FAILURE);
if (do_pid)
- return usage (EXIT_FAILURE);
- continue;
- }
-
- if (! *argv) {
- return usage (EXIT_FAILURE);
- }
- if (do_pid) {
- numsig = -1;
- }
-
- /* we're done with the options.
- the rest of the arguments should be process ids and names.
- kill them. */
- for (errors = 0; (arg = *argv) != NULL; argv++) {
- pid = strtol (arg, &ep, 10);
- if (! *ep)
- errors += kill_verbose (arg, pid, numsig);
- else {
- struct proc_processes *ps = proc_open_processes();
- int ct = 0;
-
- if (!ps)
- continue;
-
- if (!check_all)
- proc_processes_filter_by_uid(ps, getuid());
-
- proc_processes_filter_by_name(ps, arg);
-
- while (proc_next_pid(ps, &pid) == 0) {
- errors += kill_verbose(arg, pid, numsig);
- ct++;
- }
-
- if (!ct) {
- errors++;
- warnx (_("cannot find process \"%s\""), arg);
- }
- proc_close_processes(ps);
+ numsig = -1;
+
+ /* We are done with the options. The rest of the arguments
+ * should be process ids and names, kill them. */
+ for (errors = 0; (arg = *argv) != NULL; argv++) {
+ pid = strtol(arg, &ep, 10);
+ if (!*ep)
+ errors += kill_verbose(arg, pid, numsig);
+ else {
+ struct proc_processes *ps = proc_open_processes();
+ int ct = 0;
+
+ if (!ps)
+ continue;
+ if (!check_all)
+ proc_processes_filter_by_uid(ps, getuid());
+ proc_processes_filter_by_name(ps, arg);
+ while (proc_next_pid(ps, &pid) == 0) {
+ errors += kill_verbose(arg, pid, numsig);
+ ct++;
+ }
+ if (!ct) {
+ errors++;
+ warnx(_("cannot find process \"%s\""), arg);
+ }
+ proc_close_processes(ps);
+ }
}
- }
- if (errors != 0)
- errors = EXIT_FAILURE;
- return errors;
+ if (errors != 0)
+ errors = EXIT_FAILURE;
+ return errors;
}
#ifdef SIGRTMIN
@@ -323,83 +315,77 @@ static int rtsig_to_signum(char *sig)
sig += 4;
maxi = 1;
}
-
if (!isdigit(*sig))
return -1;
-
errno = 0;
num = strtol(sig, &ep, 10);
if (!ep || sig == ep || errno || num < 0)
return -1;
-
num = maxi ? SIGRTMAX - num : SIGRTMIN + num;
-
if (num < SIGRTMIN || num > SIGRTMAX)
return -1;
-
return num;
}
#endif
-static int signame_to_signum (char *sig)
+static int signame_to_signum(char *sig)
{
- size_t n;
-
- if (! strncasecmp (sig, "sig", 3))
- sig += 3;
+ size_t n;
+ if (!strncasecmp(sig, "sig", 3))
+ sig += 3;
#ifdef SIGRTMIN
- /* RT signals */
- if (!strncasecmp(sig, "rt", 2))
- return rtsig_to_signum(sig + 2);
+ /* RT signals */
+ if (!strncasecmp(sig, "rt", 2))
+ return rtsig_to_signum(sig + 2);
#endif
- /* Normal sugnals */
- for (n = 0; n < ARRAY_SIZE(sys_signame); n++) {
- if (! strcasecmp (sys_signame[n].name, sig))
- return sys_signame[n].val;
- }
- return (-1);
+ /* Normal sugnals */
+ for (n = 0; n < ARRAY_SIZE(sys_signame); n++) {
+ if (!strcasecmp(sys_signame[n].name, sig))
+ return sys_signame[n].val;
+ }
+ return (-1);
}
-static int arg_to_signum (char *arg, int maskbit)
+static int arg_to_signum(char *arg, int maskbit)
{
- int numsig;
- char *ep;
-
- if (isdigit (*arg)) {
- numsig = strtol (arg, &ep, 10);
- if (numsig >= NSIG && maskbit && (numsig & 128) != 0)
- numsig -= 128;
- if (*ep != 0 || numsig < 0 || numsig >= NSIG)
- return (-1);
- return (numsig);
- }
- return signame_to_signum (arg);
+ int numsig;
+ char *ep;
+
+ if (isdigit(*arg)) {
+ numsig = strtol(arg, &ep, 10);
+ if (numsig >= NSIG && maskbit && (numsig & 128) != 0)
+ numsig -= 128;
+ if (*ep != 0 || numsig < 0 || numsig >= NSIG)
+ return (-1);
+ return (numsig);
+ }
+ return signame_to_signum(arg);
}
-static void nosig (char *name)
+static void nosig(char *name)
{
- warnx (_("unknown signal %s; valid signals:"), name);
- printsignals (stderr, 1);
+ warnx(_("unknown signal %s; valid signals:"), name);
+ printsignals(stderr, 1);
}
-static void printsig (int sig)
+static void printsig(int sig)
{
- size_t n;
+ size_t n;
- for (n = 0; n < ARRAY_SIZE(sys_signame); n++) {
- if (sys_signame[n].val == sig) {
- printf ("%s\n", sys_signame[n].name);
- return;
+ for (n = 0; n < ARRAY_SIZE(sys_signame); n++) {
+ if (sys_signame[n].val == sig) {
+ printf("%s\n", sys_signame[n].name);
+ return;
+ }
}
- }
#ifdef SIGRTMIN
- if (sig >= SIGRTMIN && sig <= SIGRTMAX) {
- printf ("RT%d\n", sig - SIGRTMIN);
- return;
- }
+ if (sig >= SIGRTMIN && sig <= SIGRTMAX) {
+ printf("RT%d\n", sig - SIGRTMIN);
+ return;
+ }
#endif
- printf("%d\n", sig);
+ printf("%d\n", sig);
}
#define FIELD_WIDTH 11
@@ -407,50 +393,48 @@ static void pretty_print_signal(FILE *fp, size_t term_width, size_t *lpos,
int signum, const char *name)
{
if (term_width < (*lpos + FIELD_WIDTH)) {
- fputc ('\n', fp);
- *lpos = 0;
+ fputc('\n', fp);
+ *lpos = 0;
}
*lpos += FIELD_WIDTH;
- fprintf (fp, "%2d %-8s", signum, name);
+ fprintf(fp, "%2d %-8s", signum, name);
}
-static void printsignals (FILE *fp, int pretty)
+static void printsignals(FILE *fp, int pretty)
{
- size_t n, lth, lpos = 0, width;
-
- if (!pretty) {
- for (n = 0; n < ARRAY_SIZE(sys_signame); n++) {
- lth = 1+strlen(sys_signame[n].name);
- if (lpos+lth > 72) {
- fputc ('\n', fp);
- lpos = 0;
- } else if (lpos)
- fputc (' ', fp);
- lpos += lth;
- fputs (sys_signame[n].name, fp);
- }
+ size_t n, lth, lpos = 0, width;
+
+ if (!pretty) {
+ for (n = 0; n < ARRAY_SIZE(sys_signame); n++) {
+ lth = 1 + strlen(sys_signame[n].name);
+ if (lpos + lth > 72) {
+ fputc('\n', fp);
+ lpos = 0;
+ } else if (lpos)
+ fputc(' ', fp);
+ lpos += lth;
+ fputs(sys_signame[n].name, fp);
+ }
#ifdef SIGRTMIN
- fputs (" RT<N> RTMIN+<N> RTMAX-<N>", fp);
+ fputs(" RT<N> RTMIN+<N> RTMAX-<N>", fp);
#endif
- fputc ('\n', fp);
- return;
- }
- /* pretty print */
- width = get_terminal_width();
- if (width == 0)
- width = 72;
- else
- width -= 1;
-
- for (n = 0; n < ARRAY_SIZE(sys_signame); n++)
- pretty_print_signal(fp, width, &lpos,
- sys_signame[n].val, sys_signame[n].name);
-
+ fputc('\n', fp);
+ return;
+ }
+ /* pretty print */
+ width = get_terminal_width();
+ if (width == 0)
+ width = 72;
+ else
+ width -= 1;
+ for (n = 0; n < ARRAY_SIZE(sys_signame); n++)
+ pretty_print_signal(fp, width, &lpos,
+ sys_signame[n].val, sys_signame[n].name);
#ifdef SIGRTMIN
- pretty_print_signal(fp, width, &lpos, SIGRTMIN, "RTMIN");
- pretty_print_signal(fp, width, &lpos, SIGRTMAX, "RTMAX");
+ pretty_print_signal(fp, width, &lpos, SIGRTMIN, "RTMIN");
+ pretty_print_signal(fp, width, &lpos, SIGRTMAX, "RTMAX");
#endif
- fputc ('\n', fp);
+ fputc('\n', fp);
}
static int usage(int status)
@@ -471,28 +455,26 @@ static int usage(int status)
fputs(USAGE_HELP, out);
fputs(USAGE_VERSION, out);
fprintf(out, USAGE_MAN_TAIL("kill(1)"));
-
return status;
}
-static int kill_verbose (char *procname, pid_t pid, int sig)
+static int kill_verbose(char *procname, pid_t pid, int sig)
{
- int rc = 0;
+ int rc = 0;
- if (sig < 0) {
- printf ("%ld\n", (long)pid);
- return 0;
- }
+ if (sig < 0) {
+ printf("%ld\n", (long)pid);
+ return 0;
+ }
#ifdef HAVE_SIGQUEUE
- if (use_sigval)
- rc = sigqueue(pid, sig, sigdata);
- else
+ if (use_sigval)
+ rc = sigqueue(pid, sig, sigdata);
+ else
#endif
- rc = kill (pid, sig);
-
- if (rc < 0) {
- warn(_("sending signal to %s failed"), procname);
- return 1;
- }
- return 0;
+ rc = kill(pid, sig);
+ if (rc < 0) {
+ warn(_("sending signal to %s failed"), procname);
+ return 1;
+ }
+ return 0;
}
--
1.9.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH 02/10] kill: fix coding style
2014-03-09 20:30 ` [PATCH 02/10] kill: fix coding style Sami Kerola
@ 2014-04-07 9:36 ` Karel Zak
2014-04-07 14:09 ` Sami Kerola
0 siblings, 1 reply; 24+ messages in thread
From: Karel Zak @ 2014-04-07 9:36 UTC (permalink / raw)
To: Sami Kerola; +Cc: util-linux
On Sun, Mar 09, 2014 at 03:30:15PM -0500, Sami Kerola wrote:
> The kill was deprecated at the time lot of other tools got style
> unification. Now when deprecation is lifted it is time to get kill
> cleaner.
OK, all applied.
I also did some changes to the code and the man page. The most
visible change is that kill(1) now returns 64 (like mount or chcpu)
for partial success when more than one process specified.
$ kill -s 0 firefox mutt xxx; echo $?
kill: cannot find process "xxx".
64
the old version always return "1" in this situation.
Karel
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 02/10] kill: fix coding style
2014-04-07 9:36 ` Karel Zak
@ 2014-04-07 14:09 ` Sami Kerola
0 siblings, 0 replies; 24+ messages in thread
From: Sami Kerola @ 2014-04-07 14:09 UTC (permalink / raw)
To: Karel Zak; +Cc: util-linux
On 7 April 2014 10:36, Karel Zak <kzak@redhat.com> wrote:
> On Sun, Mar 09, 2014 at 03:30:15PM -0500, Sami Kerola wrote:
>> The kill was deprecated at the time lot of other tools got style
>> unification. Now when deprecation is lifted it is time to get kill
>> cleaner.
>
> OK, all applied.
>
> I also did some changes to the code and the man page. The most
> visible change is that kill(1) now returns 64 (like mount or chcpu)
> for partial success when more than one process specified.
>
> $ kill -s 0 firefox mutt xxx; echo $?
> kill: cannot find process "xxx".
> 64
>
> the old version always return "1" in this situation.
Thank you, and great that you noticed the !check_all mistake. I will
try to add more kill regression tests sometime soon.
--
Sami Kerola
http://www.iki.fi/kerolasa/
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 03/10] kill: flip all comparions to be in smaller - greater order
2014-03-09 20:30 [PATCH 01/10] findfs: use symbolic exit values, and tell about them in manual Sami Kerola
2014-03-09 20:30 ` [PATCH 02/10] kill: fix coding style Sami Kerola
@ 2014-03-09 20:30 ` Sami Kerola
2014-03-19 12:32 ` Benno Schulenberg
2014-03-09 20:30 ` [PATCH 04/10] kill: move magic numbers in beginning of the file Sami Kerola
` (9 subsequent siblings)
11 siblings, 1 reply; 24+ messages in thread
From: Sami Kerola @ 2014-03-09 20:30 UTC (permalink / raw)
To: util-linux; +Cc: kerolasa
This makes code more readable.
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
misc-utils/kill.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/misc-utils/kill.c b/misc-utils/kill.c
index e7bf5f4..877c24e 100644
--- a/misc-utils/kill.c
+++ b/misc-utils/kill.c
@@ -170,7 +170,7 @@ int main(int argc, char **argv)
/* Loop through the arguments. Actually, -a is the only option
* can be used with other options. The 'kill' is basically a
* one-option-at-most program. */
- for (argc--, argv++; argc > 0; argc--, argv++) {
+ for (argc--, argv++; 0 < argc; argc--, argv++) {
arg = *argv;
if (*arg != '-')
break;
@@ -195,7 +195,7 @@ int main(int argc, char **argv)
printsignals(stdout, 0);
return EXIT_SUCCESS;
}
- if (argc > 2)
+ if (2 < argc)
return usage(EXIT_FAILURE);
/* argc == 2, accept "kill -l $?" */
arg = argv[1];
@@ -322,7 +322,7 @@ static int rtsig_to_signum(char *sig)
if (!ep || sig == ep || errno || num < 0)
return -1;
num = maxi ? SIGRTMAX - num : SIGRTMIN + num;
- if (num < SIGRTMIN || num > SIGRTMAX)
+ if (num < SIGRTMIN || SIGRTMAX < num)
return -1;
return num;
}
@@ -354,9 +354,9 @@ static int arg_to_signum(char *arg, int maskbit)
if (isdigit(*arg)) {
numsig = strtol(arg, &ep, 10);
- if (numsig >= NSIG && maskbit && (numsig & 128) != 0)
+ if (NSIG <= numsig && maskbit && (numsig & 128) != 0)
numsig -= 128;
- if (*ep != 0 || numsig < 0 || numsig >= NSIG)
+ if (*ep != 0 || numsig < 0 || NSIG <= numsig)
return (-1);
return (numsig);
}
@@ -380,7 +380,7 @@ static void printsig(int sig)
}
}
#ifdef SIGRTMIN
- if (sig >= SIGRTMIN && sig <= SIGRTMAX) {
+ if (SIGRTMIN <= sig && sig <= SIGRTMAX) {
printf("RT%d\n", sig - SIGRTMIN);
return;
}
@@ -407,7 +407,7 @@ static void printsignals(FILE *fp, int pretty)
if (!pretty) {
for (n = 0; n < ARRAY_SIZE(sys_signame); n++) {
lth = 1 + strlen(sys_signame[n].name);
- if (lpos + lth > 72) {
+ if (72 < lpos + lth) {
fputc('\n', fp);
lpos = 0;
} else if (lpos)
--
1.9.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH 03/10] kill: flip all comparions to be in smaller - greater order
2014-03-09 20:30 ` [PATCH 03/10] kill: flip all comparions to be in smaller - greater order Sami Kerola
@ 2014-03-19 12:32 ` Benno Schulenberg
2014-03-19 19:35 ` Sami Kerola
0 siblings, 1 reply; 24+ messages in thread
From: Benno Schulenberg @ 2014-03-19 12:32 UTC (permalink / raw)
To: Sami Kerola; +Cc: Util-Linux
On Sun, Mar 9, 2014, at 21:30, Sami Kerola wrote:
> This makes code more readable.
Hmm, I don't agree with that.
> - if (argc > 2)
> + if (2 < argc)
> - if (num < SIGRTMIN || num > SIGRTMAX)
> + if (num < SIGRTMIN || SIGRTMAX < num)
I like to see always the variable on the left and the fixed value on the right.
Regards,
Benno
--
http://www.fastmail.fm - Send your email first class
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 03/10] kill: flip all comparions to be in smaller - greater order
2014-03-19 12:32 ` Benno Schulenberg
@ 2014-03-19 19:35 ` Sami Kerola
2014-03-19 20:30 ` Benno Schulenberg
0 siblings, 1 reply; 24+ messages in thread
From: Sami Kerola @ 2014-03-19 19:35 UTC (permalink / raw)
To: Benno Schulenberg; +Cc: Util-Linux
On 19 March 2014 12:32, Benno Schulenberg <bensberg@justemail.net> wrote:
> On Sun, Mar 9, 2014, at 21:30, Sami Kerola wrote:
>> This makes code more readable.
>
> Hmm, I don't agree with that.
>
>> - if (argc > 2)
>> + if (2 < argc)
>
>> - if (num < SIGRTMIN || num > SIGRTMAX)
>> + if (num < SIGRTMIN || SIGRTMAX < num)
>
> I like to see always the variable on the left and the fixed value on
> the right.
Hi Benno,
Once up on time I also thought
if (value > constant)
is better, but then I read coreutils/HACKING file and found
http://thread.gmane.org/gmane.comp.version-control.git/3903/focus=4126
which made me to change my mind. Now I believe it is best to write
comparisions in order the numerical values, something like this:
1 < 2 && 3 < 4
There are couple reasons. First of all C, as many other programming
languages, are read from left to right. When counting smaller to greater
order somehow feels correct for most of the people. To be more concrete;
lets assume a value is expected to be within range 2 - 3, in this case
if (1 < value && value < 4)
do_something();
is how I would write the comparison. When the value must not be within
that range
if (value < 2 || 3 < value)
error();
With multiple operators sticking to only one way of comparing is, IMHO,
quite much more readable than using both operators. Notice that the
alternative would look, when wrote with constants, the following.
2 > 1 && 3 < 4
Since I like to get my eyes to read code without thinking too hard I like
to see all comparision operations in numeric order, including the cases
when there is exactly one comparison, such as
if (value < 0)
That said I am aware style is matter of taste, and things of that nature
are easy to disagree. Even if that would be the case I hope you don't
recard numeric order completely loonie.
--
Sami Kerola
http://www.iki.fi/kerolasa/
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 03/10] kill: flip all comparions to be in smaller - greater order
2014-03-19 19:35 ` Sami Kerola
@ 2014-03-19 20:30 ` Benno Schulenberg
0 siblings, 0 replies; 24+ messages in thread
From: Benno Schulenberg @ 2014-03-19 20:30 UTC (permalink / raw)
To: kerolasa; +Cc: Util-Linux
Hello Sami,
On Wed, Mar 19, 2014, at 20:35, Sami Kerola wrote:
> That said I am aware style is matter of taste, and things of that nature
> are easy to disagree. Even if that would be the case I hope you don't
> recard numeric order completely loonie.
No, certainly I don't regard numeric order as completely loony. But I
do suspect there is a streak of something odd in your characters.
:)
Benno
--
http://www.fastmail.fm - Or how I learned to stop worrying and
love email again
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 04/10] kill: move magic numbers in beginning of the file
2014-03-09 20:30 [PATCH 01/10] findfs: use symbolic exit values, and tell about them in manual Sami Kerola
2014-03-09 20:30 ` [PATCH 02/10] kill: fix coding style Sami Kerola
2014-03-09 20:30 ` [PATCH 03/10] kill: flip all comparions to be in smaller - greater order Sami Kerola
@ 2014-03-09 20:30 ` Sami Kerola
2014-03-09 20:30 ` [PATCH 05/10] kill: make usage() not to return Sami Kerola
` (8 subsequent siblings)
11 siblings, 0 replies; 24+ messages in thread
From: Sami Kerola @ 2014-03-09 20:30 UTC (permalink / raw)
To: util-linux; +Cc: kerolasa
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
misc-utils/kill.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/misc-utils/kill.c b/misc-utils/kill.c
index 877c24e..b95e03d 100644
--- a/misc-utils/kill.c
+++ b/misc-utils/kill.c
@@ -57,6 +57,11 @@
#include "ttyutils.h"
#include "xalloc.h"
+enum {
+ KILL_FIELD_WIDTH = 11,
+ KILL_OUTPUT_WIDTH = 72
+};
+
struct signv {
const char *name;
int val;
@@ -388,15 +393,14 @@ static void printsig(int sig)
printf("%d\n", sig);
}
-#define FIELD_WIDTH 11
static void pretty_print_signal(FILE *fp, size_t term_width, size_t *lpos,
int signum, const char *name)
{
- if (term_width < (*lpos + FIELD_WIDTH)) {
+ if (term_width < (*lpos + KILL_FIELD_WIDTH)) {
fputc('\n', fp);
*lpos = 0;
}
- *lpos += FIELD_WIDTH;
+ *lpos += KILL_FIELD_WIDTH;
fprintf(fp, "%2d %-8s", signum, name);
}
@@ -407,7 +411,7 @@ static void printsignals(FILE *fp, int pretty)
if (!pretty) {
for (n = 0; n < ARRAY_SIZE(sys_signame); n++) {
lth = 1 + strlen(sys_signame[n].name);
- if (72 < lpos + lth) {
+ if (KILL_OUTPUT_WIDTH < lpos + lth) {
fputc('\n', fp);
lpos = 0;
} else if (lpos)
@@ -424,7 +428,7 @@ static void printsignals(FILE *fp, int pretty)
/* pretty print */
width = get_terminal_width();
if (width == 0)
- width = 72;
+ width = KILL_OUTPUT_WIDTH;
else
width -= 1;
for (n = 0; n < ARRAY_SIZE(sys_signame); n++)
--
1.9.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 05/10] kill: make usage() not to return
2014-03-09 20:30 [PATCH 01/10] findfs: use symbolic exit values, and tell about them in manual Sami Kerola
` (2 preceding siblings ...)
2014-03-09 20:30 ` [PATCH 04/10] kill: move magic numbers in beginning of the file Sami Kerola
@ 2014-03-09 20:30 ` Sami Kerola
2014-03-19 12:37 ` Benno Schulenberg
2014-03-09 20:30 ` [PATCH 06/10] kill: leeway deprecation of --pid invocation as command name Sami Kerola
` (7 subsequent siblings)
11 siblings, 1 reply; 24+ messages in thread
From: Sami Kerola @ 2014-03-09 20:30 UTC (permalink / raw)
To: util-linux; +Cc: kerolasa
And change the function argument to be output stream. Earlier the --help
made kill to exit with none zero value, that now is corrected.
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
misc-utils/kill.c | 26 ++++++++++++--------------
1 file changed, 12 insertions(+), 14 deletions(-)
diff --git a/misc-utils/kill.c b/misc-utils/kill.c
index b95e03d..2fecda3 100644
--- a/misc-utils/kill.c
+++ b/misc-utils/kill.c
@@ -149,7 +149,7 @@ static int arg_to_signum(char *arg, int mask);
static void nosig(char *name);
static void printsig(int sig);
static void printsignals(FILE *fp, int pretty);
-static int usage(int status);
+static void __attribute__((__noreturn__)) usage(FILE *out);
static int kill_verbose(char *procname, int pid, int sig);
#ifdef HAVE_SIGQUEUE
@@ -189,7 +189,7 @@ int main(int argc, char **argv)
return EXIT_SUCCESS;
}
if (!strcmp(arg, "-h") || !strcmp(arg, "--help"))
- return usage(EXIT_FAILURE);
+ usage(stdout);
if (!strcmp(arg, "-a") || !strcmp(arg, "--all")) {
check_all++;
@@ -201,7 +201,7 @@ int main(int argc, char **argv)
return EXIT_SUCCESS;
}
if (2 < argc)
- return usage(EXIT_FAILURE);
+ usage(stderr);
/* argc == 2, accept "kill -l $?" */
arg = argv[1];
if ((numsig = arg_to_signum(arg, 1)) < 0)
@@ -225,15 +225,15 @@ int main(int argc, char **argv)
if (!strcmp(arg, "-p") || !strcmp(arg, "--pid")) {
do_pid++;
if (do_kill)
- return usage(EXIT_FAILURE);
+ usage(stderr);
continue;
}
if (!strcmp(arg, "-s") || !strcmp(arg, "--signal")) {
if (argc < 2)
- return usage(EXIT_FAILURE);
+ usage(stderr);
do_kill++;
if (do_pid)
- return usage(EXIT_FAILURE);
+ usage(stderr);
argc--, argv++;
arg = *argv;
if ((numsig = arg_to_signum(arg, 0)) < 0) {
@@ -244,7 +244,7 @@ int main(int argc, char **argv)
}
if (!strcmp(arg, "-q") || !strcmp(arg, "--queue")) {
if (argc < 2)
- return usage(EXIT_FAILURE);
+ usage(stderr);
argc--, argv++;
arg = *argv;
#ifdef HAVE_SIGQUEUE
@@ -266,14 +266,14 @@ int main(int argc, char **argv)
break;
arg++;
if ((numsig = arg_to_signum(arg, 0)) < 0)
- return usage(EXIT_FAILURE);
+ usage(stderr);
do_kill++;
if (do_pid)
- return usage(EXIT_FAILURE);
+ usage(stderr);
continue;
}
if (!*argv)
- return usage(EXIT_FAILURE);
+ usage(stderr);
if (do_pid)
numsig = -1;
@@ -441,10 +441,8 @@ static void printsignals(FILE *fp, int pretty)
fputc('\n', fp);
}
-static int usage(int status)
+static void __attribute__((__noreturn__)) usage(FILE *out)
{
- FILE *out = (status == 0 ? stdout : stderr);
-
fputs(USAGE_HEADER, out);
fprintf(out, _(" %s [options] <pid|name> [...]\n"), program_invocation_short_name);
fputs(USAGE_OPTIONS, out);
@@ -459,7 +457,7 @@ static int usage(int status)
fputs(USAGE_HELP, out);
fputs(USAGE_VERSION, out);
fprintf(out, USAGE_MAN_TAIL("kill(1)"));
- return status;
+ exit(out == stderr ? EXIT_FAILURE : EXIT_SUCCESS);
}
static int kill_verbose(char *procname, pid_t pid, int sig)
--
1.9.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH 05/10] kill: make usage() not to return
2014-03-09 20:30 ` [PATCH 05/10] kill: make usage() not to return Sami Kerola
@ 2014-03-19 12:37 ` Benno Schulenberg
2014-03-19 19:02 ` Sami Kerola
0 siblings, 1 reply; 24+ messages in thread
From: Benno Schulenberg @ 2014-03-19 12:37 UTC (permalink / raw)
To: Sami Kerola; +Cc: Util-Linux
On Sun, Mar 9, 2014, at 21:30, Sami Kerola wrote:
> And change the function argument to be output stream. Earlier the --help
> made kill to exit with none zero value, that now is corrected.
Textual amendments:
s/be/be an/
s/made/option made/
s/to exit/exit/
s/none zero/non-zero/
s/now is/is now/
Or in summary:
And change the function argument to be an output stream. Earlier the
--help option made kill exit with a non-zero value; that is now corrected.
Regards,
Benno
--
http://www.fastmail.fm - Faster than the air-speed velocity of an
unladen european swallow
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 05/10] kill: make usage() not to return
2014-03-19 12:37 ` Benno Schulenberg
@ 2014-03-19 19:02 ` Sami Kerola
0 siblings, 0 replies; 24+ messages in thread
From: Sami Kerola @ 2014-03-19 19:02 UTC (permalink / raw)
To: Benno Schulenberg; +Cc: Util-Linux
On 19 March 2014 12:37, Benno Schulenberg <bensberg@justemail.net> wrote:
> On Sun, Mar 9, 2014, at 21:30, Sami Kerola wrote:
>> And change the function argument to be output stream. Earlier the --help
>> made kill to exit with none zero value, that now is corrected.
>
> Textual amendments:
>
> s/be/be an/
> s/made/option made/
> s/to exit/exit/
> s/none zero/non-zero/
> s/now is/is now/
>
> Or in summary:
>
> And change the function argument to be an output stream. Earlier the
> --help option made kill exit with a non-zero value; that is now corrected.
Thank you Benno, and Q.E.D. my English is rubbish.
Karel, that correction and the earlier fstrim exit code definition
reshuffle can be found from my git. Details for easy merge are below.
--- snip
The following changes since commit 422f93bfbb5df358fad3fedfe9eb0247cbf3cb14:
include/closestream: don't wipe errno on EPIPE (2014-03-13 12:41:03 +0100)
are available in the git repository at:
git://github.com/kerolasa/lelux-utiliteetit.git 2014wk09
for you to fetch changes up to 6a02f1dd9639b8b13c318a061951bc7b7a8b8f6e:
kill: move sigqueue inputs to control struct (2014-03-19 18:57:21 +0000)
----------------------------------------------------------------
Sami Kerola (16):
build-sys: unify function and variable attribute spacing
mcookie: use lib/randutils
mcookie: allow --file option be defined multiple times
mcookie: use same variable type consistently
mcookie: use control structure, and fix usage()
mcookie: add --max-size option
findfs: use symbolic exit values, and tell about them in manual
kill: fix coding style
kill: flip all comparions to be in smaller - greater order
kill: move magic numbers in beginning of the file
kill: make usage() not to return
kill: deprecat invocation as 'pid' command name
kill: use control structure to pass user input to functions
kill: tell what is wrong rather than output usage()
kill: add parse_arguments() function
kill: move sigqueue inputs to control struct
--
Sami Kerola
http://www.iki.fi/kerolasa/
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 06/10] kill: leeway deprecation of --pid invocation as command name
2014-03-09 20:30 [PATCH 01/10] findfs: use symbolic exit values, and tell about them in manual Sami Kerola
` (3 preceding siblings ...)
2014-03-09 20:30 ` [PATCH 05/10] kill: make usage() not to return Sami Kerola
@ 2014-03-09 20:30 ` Sami Kerola
2014-03-11 7:21 ` Bernhard Voelker
2014-04-07 8:30 ` Karel Zak
2014-03-09 20:30 ` [PATCH 07/10] kill: use control structure to pass user input to functions Sami Kerola
` (6 subsequent siblings)
11 siblings, 2 replies; 24+ messages in thread
From: Sami Kerola @ 2014-03-09 20:30 UTC (permalink / raw)
To: util-linux; +Cc: kerolasa
Who knows if anyone is linking or copying kill to a file name 'pid' and
using --pid option functionality that way, so lets inform these users and
get rid of command name hack after few releases.
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
misc-utils/kill.1 | 5 +++++
misc-utils/kill.c | 2 ++
2 files changed, 7 insertions(+)
diff --git a/misc-utils/kill.1 b/misc-utils/kill.1
index 50f5cd2..e6c4000 100644
--- a/misc-utils/kill.1
+++ b/misc-utils/kill.1
@@ -88,6 +88,11 @@ Specify that
.B kill
should only print the process id (pid) of the named processes, and not send any
signals.
+.IP
+The \-\-pid option functionality is enabled when the command
+copied or linked to name
+.BR pid .
+This functionality will be deprecated in future.
.TP
\fB\-q\fR, \fB\-\-queue\fR \fIsigval\fR
Use
diff --git a/misc-utils/kill.c b/misc-utils/kill.c
index 2fecda3..11c8cd2 100644
--- a/misc-utils/kill.c
+++ b/misc-utils/kill.c
@@ -170,6 +170,8 @@ int main(int argc, char **argv)
numsig = SIGTERM;
do_pid = (!strcmp(program_invocation_short_name, "pid")); /* Yecch */
+ if (do_pid)
+ warnx(_("use of 'kill --pid' option as command name will be deprecated in future"));
do_kill = 0;
check_all = 0;
/* Loop through the arguments. Actually, -a is the only option
--
1.9.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH 06/10] kill: leeway deprecation of --pid invocation as command name
2014-03-09 20:30 ` [PATCH 06/10] kill: leeway deprecation of --pid invocation as command name Sami Kerola
@ 2014-03-11 7:21 ` Bernhard Voelker
2014-03-11 12:13 ` Sami Kerola
2014-04-07 8:30 ` Karel Zak
1 sibling, 1 reply; 24+ messages in thread
From: Bernhard Voelker @ 2014-03-11 7:21 UTC (permalink / raw)
To: Sami Kerola, util-linux
On 03/09/2014 09:30 PM, Sami Kerola wrote:
> diff --git a/misc-utils/kill.c b/misc-utils/kill.c
> index 2fecda3..11c8cd2 100644
> --- a/misc-utils/kill.c
> +++ b/misc-utils/kill.c
> @@ -170,6 +170,8 @@ int main(int argc, char **argv)
>
> numsig = SIGTERM;
> do_pid = (!strcmp(program_invocation_short_name, "pid")); /* Yecch */
> + if (do_pid)
> + warnx(_("use of 'kill --pid' option as command name will be deprecated in future"));
You deprecate now:
s/will .* future/is deprecated/
Another minor note:
Shouldn't there be something like a
+ /* FIXME: remove in March 2016. */
together with deprecations like that?
It would be easier to find and fix then.
Have a nice day,
Berny
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 06/10] kill: leeway deprecation of --pid invocation as command name
2014-03-11 7:21 ` Bernhard Voelker
@ 2014-03-11 12:13 ` Sami Kerola
0 siblings, 0 replies; 24+ messages in thread
From: Sami Kerola @ 2014-03-11 12:13 UTC (permalink / raw)
To: Bernhard Voelker; +Cc: util-linux
On 11 March 2014 02:21, Bernhard Voelker <mail@bernhard-voelker.de> wrote:
> On 03/09/2014 09:30 PM, Sami Kerola wrote:
>> diff --git a/misc-utils/kill.c b/misc-utils/kill.c
>> index 2fecda3..11c8cd2 100644
>> --- a/misc-utils/kill.c
>> +++ b/misc-utils/kill.c
>> @@ -170,6 +170,8 @@ int main(int argc, char **argv)
>>
>> numsig = SIGTERM;
>> do_pid = (!strcmp(program_invocation_short_name, "pid")); /* Yecch */
>> + if (do_pid)
>> + warnx(_("use of 'kill --pid' option as command name will be deprecated in future"));
>
> You deprecate now:
> s/will .* future/is deprecated/
>
>
> Another minor note:
> Shouldn't there be something like a
>
> + /* FIXME: remove in March 2016. */
>
> together with deprecations like that?
> It would be easier to find and fix then.
Hi Bernhard,
Thank you for comment. Now when I think about deprecation it does
happen immediately, while removal follows later. Both the time change,
and FIXME are available in github, with corrections to two later
commits are available in the git repository at:
git://github.com/kerolasa/lelux-utiliteetit.git 2014wk09
for you to fetch changes up to 2b0dda54bda3ecd24ca1de4cfd7eb662438204cc.
--
Sami Kerola
http://www.iki.fi/kerolasa/
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 06/10] kill: leeway deprecation of --pid invocation as command name
2014-03-09 20:30 ` [PATCH 06/10] kill: leeway deprecation of --pid invocation as command name Sami Kerola
2014-03-11 7:21 ` Bernhard Voelker
@ 2014-04-07 8:30 ` Karel Zak
1 sibling, 0 replies; 24+ messages in thread
From: Karel Zak @ 2014-04-07 8:30 UTC (permalink / raw)
To: Sami Kerola; +Cc: util-linux
On Sun, Mar 09, 2014 at 03:30:19PM -0500, Sami Kerola wrote:
> Who knows if anyone is linking or copying kill to a file name 'pid' and
> using --pid option functionality that way, so lets inform these users and
> get rid of command name hack after few releases.
>
> Signed-off-by: Sami Kerola <kerolasa@iki.fi>
> ---
> misc-utils/kill.1 | 5 +++++
> misc-utils/kill.c | 2 ++
> 2 files changed, 7 insertions(+)
>
> diff --git a/misc-utils/kill.1 b/misc-utils/kill.1
> index 50f5cd2..e6c4000 100644
> --- a/misc-utils/kill.1
> +++ b/misc-utils/kill.1
> @@ -88,6 +88,11 @@ Specify that
> .B kill
> should only print the process id (pid) of the named processes, and not send any
> signals.
> +.IP
> +The \-\-pid option functionality is enabled when the command
The \-\-pid option functionality is *also* enabled when the command
I guess that the --pid is pretty valid an no deprecated at all, the
deprecated is "pid" as argv[0].
I'll fix it.
Karel
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 07/10] kill: use control structure to pass user input to functions
2014-03-09 20:30 [PATCH 01/10] findfs: use symbolic exit values, and tell about them in manual Sami Kerola
` (4 preceding siblings ...)
2014-03-09 20:30 ` [PATCH 06/10] kill: leeway deprecation of --pid invocation as command name Sami Kerola
@ 2014-03-09 20:30 ` Sami Kerola
2014-04-07 9:28 ` Karel Zak
2014-03-09 20:30 ` [PATCH 08/10] kill: tell what is wrong rather than output usage() Sami Kerola
` (5 subsequent siblings)
11 siblings, 1 reply; 24+ messages in thread
From: Sami Kerola @ 2014-03-09 20:30 UTC (permalink / raw)
To: util-linux; +Cc: kerolasa
This should make it easier to understand how the program works.
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
misc-utils/kill.c | 109 +++++++++++++++++++++++++++++-------------------------
1 file changed, 59 insertions(+), 50 deletions(-)
diff --git a/misc-utils/kill.c b/misc-utils/kill.c
index 11c8cd2..34bd00e 100644
--- a/misc-utils/kill.c
+++ b/misc-utils/kill.c
@@ -62,6 +62,16 @@ enum {
KILL_OUTPUT_WIDTH = 72
};
+struct kill_control {
+ char *arg;
+ pid_t pid;
+ int numsig;
+ unsigned int
+ check_all:1,
+ do_kill:1,
+ do_pid:1;
+};
+
struct signv {
const char *name;
int val;
@@ -147,10 +157,10 @@ struct signv {
static int arg_to_signum(char *arg, int mask);
static void nosig(char *name);
-static void printsig(int sig);
+static void printsig(const struct kill_control *ctl);
static void printsignals(FILE *fp, int pretty);
static void __attribute__((__noreturn__)) usage(FILE *out);
-static int kill_verbose(char *procname, int pid, int sig);
+static int kill_verbose(const struct kill_control *ctl);
#ifdef HAVE_SIGQUEUE
static int use_sigval;
@@ -159,21 +169,20 @@ static union sigval sigdata;
int main(int argc, char **argv)
{
- int errors, numsig, pid;
- char *ep, *arg;
- int do_pid, do_kill, check_all;
+ struct kill_control ctl;
+ char *arg;
+ int errors = EXIT_SUCCESS;
setlocale(LC_ALL, "");
bindtextdomain(PACKAGE, LOCALEDIR);
textdomain(PACKAGE);
atexit(close_stdout);
+ memset(&ctl, 0, sizeof(ctl));
- numsig = SIGTERM;
- do_pid = (!strcmp(program_invocation_short_name, "pid")); /* Yecch */
- if (do_pid)
+ ctl.numsig = SIGTERM;
+ ctl.do_pid = (!strcmp(program_invocation_short_name, "pid")); /* Yecch */
+ if (ctl.do_pid)
warnx(_("use of 'kill --pid' option as command name will be deprecated in future"));
- do_kill = 0;
- check_all = 0;
/* Loop through the arguments. Actually, -a is the only option
* can be used with other options. The 'kill' is basically a
* one-option-at-most program. */
@@ -194,7 +203,7 @@ int main(int argc, char **argv)
usage(stdout);
if (!strcmp(arg, "-a") || !strcmp(arg, "--all")) {
- check_all++;
+ ctl.check_all = 1;
continue;
}
if (!strcmp(arg, "-l") || !strcmp(arg, "--list")) {
@@ -206,18 +215,18 @@ int main(int argc, char **argv)
usage(stderr);
/* argc == 2, accept "kill -l $?" */
arg = argv[1];
- if ((numsig = arg_to_signum(arg, 1)) < 0)
+ if ((ctl.numsig = arg_to_signum(arg, 1)) < 0)
errx(EXIT_FAILURE, _("unknown signal: %s"),
arg);
- printsig(numsig);
+ printsig(&ctl);
return EXIT_SUCCESS;
}
/* for compatibility with procps kill(1) */
if (!strncmp(arg, "--list=", 7) || !strncmp(arg, "-l=", 3)) {
char *p = strchr(arg, '=') + 1;
- if ((numsig = arg_to_signum(p, 1)) < 0)
+ if ((ctl.numsig = arg_to_signum(p, 1)) < 0)
errx(EXIT_FAILURE, _("unknown signal: %s"), p);
- printsig(numsig);
+ printsig(&ctl);
return EXIT_SUCCESS;
}
if (!strcmp(arg, "-L") || !strcmp(arg, "--table")) {
@@ -225,20 +234,20 @@ int main(int argc, char **argv)
return EXIT_SUCCESS;
}
if (!strcmp(arg, "-p") || !strcmp(arg, "--pid")) {
- do_pid++;
- if (do_kill)
+ ctl.do_pid = 1;
+ if (ctl.do_kill)
usage(stderr);
continue;
}
if (!strcmp(arg, "-s") || !strcmp(arg, "--signal")) {
if (argc < 2)
usage(stderr);
- do_kill++;
- if (do_pid)
+ ctl.do_kill = 1;
+ if (ctl.do_pid)
usage(stderr);
argc--, argv++;
arg = *argv;
- if ((numsig = arg_to_signum(arg, 0)) < 0) {
+ if ((ctl.numsig = arg_to_signum(arg, 0)) < 0) {
nosig(arg);
return EXIT_FAILURE;
}
@@ -264,49 +273,49 @@ int main(int argc, char **argv)
* number). In case of doubt POSIX tells us to assume a
* signal. If a signal has been parsed, assume it is a
* pid, break. */
- if (do_kill)
+ if (ctl.do_kill)
break;
arg++;
- if ((numsig = arg_to_signum(arg, 0)) < 0)
+ if ((ctl.numsig = arg_to_signum(arg, 0)) < 0)
usage(stderr);
- do_kill++;
- if (do_pid)
+ ctl.do_kill = 1;
+ if (ctl.do_pid)
usage(stderr);
continue;
}
if (!*argv)
usage(stderr);
- if (do_pid)
- numsig = -1;
+ if (ctl.do_pid)
+ ctl.numsig = -1;
/* We are done with the options. The rest of the arguments
* should be process ids and names, kill them. */
- for (errors = 0; (arg = *argv) != NULL; argv++) {
- pid = strtol(arg, &ep, 10);
+ for (/* nothing */; (ctl.arg = *argv) != NULL; argv++) {
+ char *ep;
+
+ ctl.pid = strtol(ctl.arg, &ep, 10);
if (!*ep)
- errors += kill_verbose(arg, pid, numsig);
+ errors |= kill_verbose(&ctl);
else {
struct proc_processes *ps = proc_open_processes();
int ct = 0;
if (!ps)
continue;
- if (!check_all)
+ if (ctl.check_all)
proc_processes_filter_by_uid(ps, getuid());
- proc_processes_filter_by_name(ps, arg);
- while (proc_next_pid(ps, &pid) == 0) {
- errors += kill_verbose(arg, pid, numsig);
+ proc_processes_filter_by_name(ps, ctl.arg);
+ while (proc_next_pid(ps, &(ctl.pid)) == 0) {
+ errors |= kill_verbose(&ctl);
ct++;
}
if (!ct) {
- errors++;
- warnx(_("cannot find process \"%s\""), arg);
+ errors = EXIT_FAILURE;
+ warnx(_("cannot find process \"%s\""), ctl.arg);
}
proc_close_processes(ps);
}
}
- if (errors != 0)
- errors = EXIT_FAILURE;
return errors;
}
@@ -376,23 +385,23 @@ static void nosig(char *name)
printsignals(stderr, 1);
}
-static void printsig(int sig)
+static void printsig(const struct kill_control *ctl)
{
size_t n;
for (n = 0; n < ARRAY_SIZE(sys_signame); n++) {
- if (sys_signame[n].val == sig) {
+ if (sys_signame[n].val == ctl->numsig) {
printf("%s\n", sys_signame[n].name);
return;
}
}
#ifdef SIGRTMIN
- if (SIGRTMIN <= sig && sig <= SIGRTMAX) {
- printf("RT%d\n", sig - SIGRTMIN);
+ if (SIGRTMIN <= ctl->numsig && ctl->numsig <= SIGRTMAX) {
+ printf("RT%d\n", ctl->numsig - SIGRTMIN);
return;
}
#endif
- printf("%d\n", sig);
+ printf("%d\n", ctl->numsig);
}
static void pretty_print_signal(FILE *fp, size_t term_width, size_t *lpos,
@@ -462,23 +471,23 @@ static void __attribute__((__noreturn__)) usage(FILE *out)
exit(out == stderr ? EXIT_FAILURE : EXIT_SUCCESS);
}
-static int kill_verbose(char *procname, pid_t pid, int sig)
+static int kill_verbose(const struct kill_control *ctl)
{
int rc = 0;
- if (sig < 0) {
- printf("%ld\n", (long)pid);
+ if (ctl->numsig < 0) {
+ printf("%ld\n", (long) ctl->pid);
return 0;
}
#ifdef HAVE_SIGQUEUE
if (use_sigval)
- rc = sigqueue(pid, sig, sigdata);
+ rc = sigqueue(ctl->pid, ctl->numsig, sigdata);
else
#endif
- rc = kill(pid, sig);
+ rc = kill(ctl->pid, ctl->numsig);
if (rc < 0) {
- warn(_("sending signal to %s failed"), procname);
- return 1;
+ warn(_("sending signal to %s failed"), ctl->arg);
+ return EXIT_FAILURE;
}
- return 0;
+ return EXIT_SUCCESS;
}
--
1.9.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 08/10] kill: tell what is wrong rather than output usage()
2014-03-09 20:30 [PATCH 01/10] findfs: use symbolic exit values, and tell about them in manual Sami Kerola
` (5 preceding siblings ...)
2014-03-09 20:30 ` [PATCH 07/10] kill: use control structure to pass user input to functions Sami Kerola
@ 2014-03-09 20:30 ` Sami Kerola
2014-03-09 20:30 ` [PATCH 09/10] kill: add parse_arguments() function Sami Kerola
` (4 subsequent siblings)
11 siblings, 0 replies; 24+ messages in thread
From: Sami Kerola @ 2014-03-09 20:30 UTC (permalink / raw)
To: util-linux; +Cc: kerolasa
Getting usage as error message is not specific enough. As a user I want
to know what is wrong, and if it is unclear after error message how to
recover then I run command with --help.
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
misc-utils/kill.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/misc-utils/kill.c b/misc-utils/kill.c
index 34bd00e..30c209f 100644
--- a/misc-utils/kill.c
+++ b/misc-utils/kill.c
@@ -212,7 +212,7 @@ int main(int argc, char **argv)
return EXIT_SUCCESS;
}
if (2 < argc)
- usage(stderr);
+ errx(EXIT_FAILURE, _("too many arguments"));
/* argc == 2, accept "kill -l $?" */
arg = argv[1];
if ((ctl.numsig = arg_to_signum(arg, 1)) < 0)
@@ -236,15 +236,15 @@ int main(int argc, char **argv)
if (!strcmp(arg, "-p") || !strcmp(arg, "--pid")) {
ctl.do_pid = 1;
if (ctl.do_kill)
- usage(stderr);
+ errx(EXIT_FAILURE, _("%s and %s are mutually exclusive"), "--pid", "--signal");
continue;
}
if (!strcmp(arg, "-s") || !strcmp(arg, "--signal")) {
if (argc < 2)
- usage(stderr);
+ errx(EXIT_FAILURE, _("not enough arguments"));
ctl.do_kill = 1;
if (ctl.do_pid)
- usage(stderr);
+ errx(EXIT_FAILURE, _("%s and %s are mutually exclusive"), "--pid", "--signal");
argc--, argv++;
arg = *argv;
if ((ctl.numsig = arg_to_signum(arg, 0)) < 0) {
@@ -255,7 +255,7 @@ int main(int argc, char **argv)
}
if (!strcmp(arg, "-q") || !strcmp(arg, "--queue")) {
if (argc < 2)
- usage(stderr);
+ errx(EXIT_FAILURE, _("option '%s' requires an argument"), arg);
argc--, argv++;
arg = *argv;
#ifdef HAVE_SIGQUEUE
@@ -277,14 +277,14 @@ int main(int argc, char **argv)
break;
arg++;
if ((ctl.numsig = arg_to_signum(arg, 0)) < 0)
- usage(stderr);
+ errx(EXIT_FAILURE, _("invalid sigval argument"));
ctl.do_kill = 1;
if (ctl.do_pid)
- usage(stderr);
+ errx(EXIT_FAILURE, _("%s and %s are mutually exclusive"), "--pid", "--signal");
continue;
}
if (!*argv)
- usage(stderr);
+ errx(EXIT_FAILURE, _("not enough arguments"));
if (ctl.do_pid)
ctl.numsig = -1;
--
1.9.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 09/10] kill: add parse_arguments() function
2014-03-09 20:30 [PATCH 01/10] findfs: use symbolic exit values, and tell about them in manual Sami Kerola
` (6 preceding siblings ...)
2014-03-09 20:30 ` [PATCH 08/10] kill: tell what is wrong rather than output usage() Sami Kerola
@ 2014-03-09 20:30 ` Sami Kerola
2014-03-09 20:30 ` [PATCH 10/10] kill: move sigqueue inputs to control struct Sami Kerola
` (3 subsequent siblings)
11 siblings, 0 replies; 24+ messages in thread
From: Sami Kerola @ 2014-03-09 20:30 UTC (permalink / raw)
To: util-linux; +Cc: kerolasa
Long main() is difficult to read, so moving argument parsing to separate
function should make sense.
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
misc-utils/kill.c | 113 +++++++++++++++++++++++++++++-------------------------
1 file changed, 60 insertions(+), 53 deletions(-)
diff --git a/misc-utils/kill.c b/misc-utils/kill.c
index 30c209f..e5ec01e 100644
--- a/misc-utils/kill.c
+++ b/misc-utils/kill.c
@@ -155,6 +155,7 @@ struct signv {
#endif
};
+static char **parse_arguments(int argc, char **argv, struct kill_control *ctl);
static int arg_to_signum(char *arg, int mask);
static void nosig(char *name);
static void printsig(const struct kill_control *ctl);
@@ -170,7 +171,6 @@ static union sigval sigdata;
int main(int argc, char **argv)
{
struct kill_control ctl;
- char *arg;
int errors = EXIT_SUCCESS;
setlocale(LC_ALL, "");
@@ -183,6 +183,42 @@ int main(int argc, char **argv)
ctl.do_pid = (!strcmp(program_invocation_short_name, "pid")); /* Yecch */
if (ctl.do_pid)
warnx(_("use of 'kill --pid' option as command name will be deprecated in future"));
+ argv = parse_arguments(argc, argv, &ctl);
+ /* We are done with the options. The rest of the arguments
+ * should be process ids and names, kill them. */
+ for (/* nothing */; (ctl.arg = *argv) != NULL; argv++) {
+ char *ep;
+
+ ctl.pid = strtol(ctl.arg, &ep, 10);
+ if (!*ep)
+ errors |= kill_verbose(&ctl);
+ else {
+ struct proc_processes *ps = proc_open_processes();
+ int ct = 0;
+
+ if (!ps)
+ continue;
+ if (ctl.check_all)
+ proc_processes_filter_by_uid(ps, getuid());
+ proc_processes_filter_by_name(ps, ctl.arg);
+ while (proc_next_pid(ps, &(ctl.pid)) == 0) {
+ errors |= kill_verbose(&ctl);
+ ct++;
+ }
+ if (!ct) {
+ errors = EXIT_FAILURE;
+ warnx(_("cannot find process \"%s\""), ctl.arg);
+ }
+ proc_close_processes(ps);
+ }
+ }
+ return errors;
+}
+
+static char **parse_arguments(int argc, char **argv, struct kill_control *ctl)
+{
+ char *arg;
+
/* Loop through the arguments. Actually, -a is the only option
* can be used with other options. The 'kill' is basically a
* one-option-at-most program. */
@@ -197,59 +233,59 @@ int main(int argc, char **argv)
if (!strcmp(arg, "-v") || !strcmp(arg, "-V") ||
!strcmp(arg, "--version")) {
printf(UTIL_LINUX_VERSION);
- return EXIT_SUCCESS;
+ exit(EXIT_SUCCESS);
}
if (!strcmp(arg, "-h") || !strcmp(arg, "--help"))
usage(stdout);
if (!strcmp(arg, "-a") || !strcmp(arg, "--all")) {
- ctl.check_all = 1;
+ ctl->check_all = 1;
continue;
}
if (!strcmp(arg, "-l") || !strcmp(arg, "--list")) {
if (argc < 2) {
printsignals(stdout, 0);
- return EXIT_SUCCESS;
+ exit(EXIT_SUCCESS);
}
if (2 < argc)
errx(EXIT_FAILURE, _("too many arguments"));
/* argc == 2, accept "kill -l $?" */
arg = argv[1];
- if ((ctl.numsig = arg_to_signum(arg, 1)) < 0)
+ if ((ctl->numsig = arg_to_signum(arg, 1)) < 0)
errx(EXIT_FAILURE, _("unknown signal: %s"),
arg);
- printsig(&ctl);
- return EXIT_SUCCESS;
+ printsig(ctl);
+ exit(EXIT_SUCCESS);
}
/* for compatibility with procps kill(1) */
if (!strncmp(arg, "--list=", 7) || !strncmp(arg, "-l=", 3)) {
char *p = strchr(arg, '=') + 1;
- if ((ctl.numsig = arg_to_signum(p, 1)) < 0)
+ if ((ctl->numsig = arg_to_signum(p, 1)) < 0)
errx(EXIT_FAILURE, _("unknown signal: %s"), p);
- printsig(&ctl);
- return EXIT_SUCCESS;
+ printsig(ctl);
+ exit(EXIT_SUCCESS);
}
if (!strcmp(arg, "-L") || !strcmp(arg, "--table")) {
printsignals(stdout, 1);
- return EXIT_SUCCESS;
+ exit(EXIT_SUCCESS);
}
if (!strcmp(arg, "-p") || !strcmp(arg, "--pid")) {
- ctl.do_pid = 1;
- if (ctl.do_kill)
+ ctl->do_pid = 1;
+ if (ctl->do_kill)
errx(EXIT_FAILURE, _("%s and %s are mutually exclusive"), "--pid", "--signal");
continue;
}
if (!strcmp(arg, "-s") || !strcmp(arg, "--signal")) {
if (argc < 2)
errx(EXIT_FAILURE, _("not enough arguments"));
- ctl.do_kill = 1;
- if (ctl.do_pid)
+ ctl->do_kill = 1;
+ if (ctl->do_pid)
errx(EXIT_FAILURE, _("%s and %s are mutually exclusive"), "--pid", "--signal");
argc--, argv++;
arg = *argv;
- if ((ctl.numsig = arg_to_signum(arg, 0)) < 0) {
+ if ((ctl->numsig = arg_to_signum(arg, 0)) < 0) {
nosig(arg);
- return EXIT_FAILURE;
+ exit(EXIT_FAILURE);
}
continue;
}
@@ -273,50 +309,21 @@ int main(int argc, char **argv)
* number). In case of doubt POSIX tells us to assume a
* signal. If a signal has been parsed, assume it is a
* pid, break. */
- if (ctl.do_kill)
+ if (ctl->do_kill)
break;
arg++;
- if ((ctl.numsig = arg_to_signum(arg, 0)) < 0)
+ if ((ctl->numsig = arg_to_signum(arg, 0)) < 0)
errx(EXIT_FAILURE, _("invalid sigval argument"));
- ctl.do_kill = 1;
- if (ctl.do_pid)
+ ctl->do_kill = 1;
+ if (ctl->do_pid)
errx(EXIT_FAILURE, _("%s and %s are mutually exclusive"), "--pid", "--signal");
continue;
}
if (!*argv)
errx(EXIT_FAILURE, _("not enough arguments"));
- if (ctl.do_pid)
- ctl.numsig = -1;
-
- /* We are done with the options. The rest of the arguments
- * should be process ids and names, kill them. */
- for (/* nothing */; (ctl.arg = *argv) != NULL; argv++) {
- char *ep;
-
- ctl.pid = strtol(ctl.arg, &ep, 10);
- if (!*ep)
- errors |= kill_verbose(&ctl);
- else {
- struct proc_processes *ps = proc_open_processes();
- int ct = 0;
-
- if (!ps)
- continue;
- if (ctl.check_all)
- proc_processes_filter_by_uid(ps, getuid());
- proc_processes_filter_by_name(ps, ctl.arg);
- while (proc_next_pid(ps, &(ctl.pid)) == 0) {
- errors |= kill_verbose(&ctl);
- ct++;
- }
- if (!ct) {
- errors = EXIT_FAILURE;
- warnx(_("cannot find process \"%s\""), ctl.arg);
- }
- proc_close_processes(ps);
- }
- }
- return errors;
+ if (ctl->do_pid)
+ ctl->numsig = -1;
+ return argv;
}
#ifdef SIGRTMIN
--
1.9.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 10/10] kill: move sigqueue inputs to control struct
2014-03-09 20:30 [PATCH 01/10] findfs: use symbolic exit values, and tell about them in manual Sami Kerola
` (7 preceding siblings ...)
2014-03-09 20:30 ` [PATCH 09/10] kill: add parse_arguments() function Sami Kerola
@ 2014-03-09 20:30 ` Sami Kerola
2014-03-12 10:37 ` [PATCH 01/10] findfs: use symbolic exit values, and tell about them in manual Karel Zak
` (2 subsequent siblings)
11 siblings, 0 replies; 24+ messages in thread
From: Sami Kerola @ 2014-03-09 20:30 UTC (permalink / raw)
To: util-linux; +Cc: kerolasa
Use of global variables is messy. The earlier implementation also
assumed queue argument never to be textual, such as 'HUP', which now
works as one might expect.
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
misc-utils/kill.c | 29 +++++++++++++++++------------
1 file changed, 17 insertions(+), 12 deletions(-)
diff --git a/misc-utils/kill.c b/misc-utils/kill.c
index e5ec01e..40f354c 100644
--- a/misc-utils/kill.c
+++ b/misc-utils/kill.c
@@ -66,10 +66,14 @@ struct kill_control {
char *arg;
pid_t pid;
int numsig;
+#ifdef HAVE_SIGQUEUE
+ union sigval sigdata;
+#endif
unsigned int
check_all:1,
do_kill:1,
- do_pid:1;
+ do_pid:1,
+ use_sigval:1;
};
struct signv {
@@ -163,10 +167,6 @@ static void printsignals(FILE *fp, int pretty);
static void __attribute__((__noreturn__)) usage(FILE *out);
static int kill_verbose(const struct kill_control *ctl);
-#ifdef HAVE_SIGQUEUE
-static int use_sigval;
-static union sigval sigdata;
-#endif
int main(int argc, char **argv)
{
@@ -289,18 +289,21 @@ static char **parse_arguments(int argc, char **argv, struct kill_control *ctl)
}
continue;
}
+#ifdef HAVE_SIGQUEUE
if (!strcmp(arg, "-q") || !strcmp(arg, "--queue")) {
if (argc < 2)
errx(EXIT_FAILURE, _("option '%s' requires an argument"), arg);
argc--, argv++;
arg = *argv;
-#ifdef HAVE_SIGQUEUE
- sigdata.sival_int =
- strtos32_or_err(arg, _("invalid sigval argument"));
- use_sigval = 1;
-#endif
+ if ((ctl->numsig = arg_to_signum(arg, 0)) < 0) {
+ nosig(arg);
+ exit(EXIT_FAILURE);
+ }
+ ctl->sigdata.sival_int = ctl->numsig;
+ ctl->use_sigval = 1;
continue;
}
+#endif
/* 'arg' begins with a dash but is not a known option.
* So it's probably something like -HUP, or -1/-n try to
* deal with it.
@@ -467,7 +470,9 @@ static void __attribute__((__noreturn__)) usage(FILE *out)
fputs(_(" -a, --all do not restrict the name-to-pid conversion to processes\n"
" with the same uid as the present process\n"), out);
fputs(_(" -s, --signal <sig> send specified signal\n"), out);
+#ifdef HAVE_SIGQUEUE
fputs(_(" -q, --queue <sig> use sigqueue(2) rather than kill(2)\n"), out);
+#endif
fputs(_(" -p, --pid print pids without signaling them\n"), out);
fputs(_(" -l, --list [=<signal>] list signal names, or convert one to a name\n"), out);
fputs(_(" -L, --table list signal names and numbers\n"), out);
@@ -487,8 +492,8 @@ static int kill_verbose(const struct kill_control *ctl)
return 0;
}
#ifdef HAVE_SIGQUEUE
- if (use_sigval)
- rc = sigqueue(ctl->pid, ctl->numsig, sigdata);
+ if (ctl->use_sigval)
+ rc = sigqueue(ctl->pid, ctl->numsig, ctl->sigdata);
else
#endif
rc = kill(ctl->pid, ctl->numsig);
--
1.9.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH 01/10] findfs: use symbolic exit values, and tell about them in manual
2014-03-09 20:30 [PATCH 01/10] findfs: use symbolic exit values, and tell about them in manual Sami Kerola
` (8 preceding siblings ...)
2014-03-09 20:30 ` [PATCH 10/10] kill: move sigqueue inputs to control struct Sami Kerola
@ 2014-03-12 10:37 ` Karel Zak
2014-03-13 16:49 ` Sami Kerola
2014-03-26 11:19 ` Karel Zak
11 siblings, 0 replies; 24+ messages in thread
From: Karel Zak @ 2014-03-12 10:37 UTC (permalink / raw)
To: Sami Kerola; +Cc: util-linux
On Sun, Mar 09, 2014 at 03:30:14PM -0500, Sami Kerola wrote:
> Signed-off-by: Sami Kerola <kerolasa@iki.fi>
> ---
> include/exitcodes.h | 5 +++++
> misc-utils/findfs.8 | 30 ++++++++++++++++++++++++++----
> misc-utils/findfs.c | 13 +++++++------
> 3 files changed, 38 insertions(+), 10 deletions(-)
>
> diff --git a/include/exitcodes.h b/include/exitcodes.h
> index 24ee123..fc893f1 100644
> --- a/include/exitcodes.h
> +++ b/include/exitcodes.h
> @@ -32,4 +32,9 @@
> #define MOUNT_EX_FAIL 32 /* mount failure */
> #define MOUNT_EX_SOMEOK 64 /* some mount succeeded */
>
> +/* Exit codes used by findfs. */
> +#define FINDFS_SUCCESS 0 /* no errors */
> +#define FINDFS_NOT_FOUND 1 /* label or uuid cannot be found */
> +#define FINDFS_USAGE_ERROR 2 /* user did something unexpected */
Do we really need to add findfs exit codes to exitcodes.h? The
codes are specific to findfs and no shared anywhere.
It would be also better to use FINDFS_EX_{SUCCESS,NOTFOUND,USAGE}.
Karel
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 01/10] findfs: use symbolic exit values, and tell about them in manual
2014-03-09 20:30 [PATCH 01/10] findfs: use symbolic exit values, and tell about them in manual Sami Kerola
` (9 preceding siblings ...)
2014-03-12 10:37 ` [PATCH 01/10] findfs: use symbolic exit values, and tell about them in manual Karel Zak
@ 2014-03-13 16:49 ` Sami Kerola
2014-03-26 11:19 ` Karel Zak
11 siblings, 0 replies; 24+ messages in thread
From: Sami Kerola @ 2014-03-13 16:49 UTC (permalink / raw)
To: util-linux, Karel Zak; +Cc: kerolasa
On 12 March 2014 10:37, Karel Zak <kzak@redhat.com> wrote:
> Do we really need to add findfs exit codes to exitcodes.h? The
> codes are specific to findfs and no shared anywhere.
>
> It would be also better to use FINDFS_EX_{SUCCESS,NOTFOUND,USAGE}.
Hi Karel,
Oh, I see. I misunderstood exitcodes.h, the modified change adds a hint
to the file how to use it.
BTW I am 98.5% certain the findfs was the last utility to use none binary
exit values and have them as numbers in source. Getting rid of such
magic was the original reason of the change.
--->8----
From: Sami Kerola <kerolasa@iki.fi>
Date: Sun, 9 Mar 2014 10:49:44 -0500
Subject: [PATCH] findfs: use symbolic exit values, and tell about them in manual
Organization: lastminute.com
And mention the exitcodes.h is meant to be used when same exit codes are
used in multiple utilities. See reference for advice about this.
Reference: http://www.spinics.net/lists/util-linux-ng/msg08970.html
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
include/exitcodes.h | 1 +
misc-utils/findfs.8 | 30 ++++++++++++++++++++++++++----
misc-utils/findfs.c | 17 +++++++++++------
3 files changed, 38 insertions(+), 10 deletions(-)
diff --git a/include/exitcodes.h b/include/exitcodes.h
index 24ee123..b7f37cd 100644
--- a/include/exitcodes.h
+++ b/include/exitcodes.h
@@ -3,6 +3,7 @@
/*
* BE CAREFUL
*
+ * Exit codes in this file are shared in multiple utilities.
* These exit codes are part of the official interface for mount,
* fsck, mkfs, etc. wrappers.
*/
diff --git a/misc-utils/findfs.8 b/misc-utils/findfs.8
index 8a6bca1..1469df2 100644
--- a/misc-utils/findfs.8
+++ b/misc-utils/findfs.8
@@ -2,7 +2,7 @@
.\" Copyright 1993, 1994, 1995 by Theodore Ts'o. All Rights Reserved.
.\" This file may be copied under the terms of the GNU Public License.
.\"
-.TH FINDFS 8 "February 2009" "util-linux" "System Administration"
+.TH FINDFS 8 "March 2014" "util-linux" "System Administration"
.SH NAME
findfs \- find a filesystem by label or UUID
.SH SYNOPSIS
@@ -21,10 +21,30 @@ or a UUID equal to
If the filesystem is found, the device name for the filesystem will
be printed on stdout.
.PP
+.SH "EXIT STATUS"
+.RS
+.PD 0
+.TP
+.B 0
+success
+.TP
+.B 1
+label or uuid cannot be found
+.TP
+.B 2
+usage error, wrong number of arguments or unknown option
+.PD
+.RE
.SH AUTHOR
.B findfs
-was originally written by Theodore Ts'o (tytso@mit.edu) and re-written for
-the util-linux package by Karel Zak (kzak@redhat.com).
+was originally written by
+.MT tytso@mit.edu
+Theodore Ts'o
+.ME
+and re-written for the util-linux package by
+.MT kzak@redhat.com
+Karel Zak
+.ME .
.SH ENVIRONMENT
.IP LIBBLKID_DEBUG=0xffff
enables debug output.
@@ -33,4 +53,6 @@ enables debug output.
.BR fsck (8)
.SH AVAILABILITY
The findfs command is part of the util-linux package and is available from
-ftp://ftp.kernel.org/pub/linux/utils/util-linux/.
+.UR ftp://\:ftp.kernel.org\:/pub\:/linux\:/utils\:/util-linux/
+Linux Kernel Archive
+.UE .
diff --git a/misc-utils/findfs.c b/misc-utils/findfs.c
index bc4a843..5c834cc 100644
--- a/misc-utils/findfs.c
+++ b/misc-utils/findfs.c
@@ -14,6 +14,11 @@
#include "nls.h"
#include "closestream.h"
#include "c.h"
+#include "exitcodes.h"
+
+#define FINDFS_EX_SUCCESS 0 /* no errors */
+#define FINDFS_EX_NOTFOUND 1 /* label or uuid cannot be found */
+#define FINDFS_EX_USAGE 2 /* user did something unexpected */
static void __attribute__((__noreturn__)) usage(int rc)
{
@@ -41,7 +46,7 @@ int main(int argc, char **argv)
if (argc != 2)
/* we return '2' for backward compatibility
* with version from e2fsprogs */
- usage(2);
+ usage(FINDFS_EX_USAGE);
if (!strncmp(argv[1], "LABEL=", 6)) {
tk = "LABEL";
@@ -52,18 +57,18 @@ int main(int argc, char **argv)
} else if (strcmp(argv[1], "-V") == 0 ||
strcmp(argv[1], "--version") == 0) {
printf(UTIL_LINUX_VERSION);
- return EXIT_SUCCESS;
+ return FINDFS_EX_SUCCESS;
} else if (strcmp(argv[1], "-h") == 0 ||
strcmp(argv[1], "--help") == 0) {
- usage(EXIT_SUCCESS);
+ usage(FINDFS_EX_SUCCESS);
} else
- usage(2);
+ usage(FINDFS_EX_USAGE);
dev = blkid_evaluate_tag(tk, vl, NULL);
if (!dev)
- errx(EXIT_FAILURE, _("unable to resolve '%s'"), argv[1]);
+ errx(FINDFS_EX_NOTFOUND, _("unable to resolve '%s'"), argv[1]);
puts(dev);
- exit(EXIT_SUCCESS);
+ return FINDFS_EX_SUCCESS;
}
--
1.9.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH 01/10] findfs: use symbolic exit values, and tell about them in manual
2014-03-09 20:30 [PATCH 01/10] findfs: use symbolic exit values, and tell about them in manual Sami Kerola
` (10 preceding siblings ...)
2014-03-13 16:49 ` Sami Kerola
@ 2014-03-26 11:19 ` Karel Zak
11 siblings, 0 replies; 24+ messages in thread
From: Karel Zak @ 2014-03-26 11:19 UTC (permalink / raw)
To: Sami Kerola; +Cc: util-linux
On Sun, Mar 09, 2014 at 03:30:14PM -0500, Sami Kerola wrote:
> include/exitcodes.h | 5 +++++
> misc-utils/findfs.8 | 30 ++++++++++++++++++++++++++----
> misc-utils/findfs.c | 13 +++++++------
> 3 files changed, 38 insertions(+), 10 deletions(-)
Applied, thanks.
> --- a/include/exitcodes.h
> +++ b/include/exitcodes.h
> @@ -32,4 +32,9 @@
> #define MOUNT_EX_FAIL 32 /* mount failure */
> #define MOUNT_EX_SOMEOK 64 /* some mount succeeded */
>
> +/* Exit codes used by findfs. */
> +#define FINDFS_SUCCESS 0 /* no errors */
> +#define FINDFS_NOT_FOUND 1 /* label or uuid cannot be found */
> +#define FINDFS_USAGE_ERROR 2 /* user did something unexpected */
I don't think it's necessary to have the codes in include/ as it is
not shared between more utils. Fixed.
Karel
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply [flat|nested] 24+ messages in thread