[PATCH 01/10] findfs: use symbolic exit values, and tell about them in manual

[PATCH 01/10] findfs: use symbolic exit values, and tell about them in manual

[Sami Kerola]

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 */
+
 #endif	/* UTIL_LINUX_EXITCODES_H */
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..aff6ba8 100644
--- a/misc-utils/findfs.c
+++ b/misc-utils/findfs.c
@@ -14,6 +14,7 @@
 #include "nls.h"
 #include "closestream.h"
 #include "c.h"
+#include "exitcodes.h"
 
 static void __attribute__((__noreturn__)) usage(int rc)
 {
@@ -41,7 +42,7 @@ int main(int argc, char **argv)
 	if (argc != 2)
 		/* we return '2' for backward compatibility
 		 * with version from e2fsprogs */
-		usage(2);
+		usage(FINDFS_USAGE_ERROR);
 
 	if (!strncmp(argv[1], "LABEL=", 6)) {
 		tk = "LABEL";
@@ -52,18 +53,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_SUCCESS;
 	} else if (strcmp(argv[1], "-h") == 0 ||
 		   strcmp(argv[1], "--help") == 0) {
-		usage(EXIT_SUCCESS);
+		usage(FINDFS_SUCCESS);
 	} else
-		usage(2);
+		usage(FINDFS_USAGE_ERROR);
 
 	dev = blkid_evaluate_tag(tk, vl, NULL);
 	if (!dev)
-		errx(EXIT_FAILURE, _("unable to resolve '%s'"),	argv[1]);
+		errx(FINDFS_NOT_FOUND, _("unable to resolve '%s'"), argv[1]);
 
 	puts(dev);
-	exit(EXIT_SUCCESS);
+	return FINDFS_SUCCESS;
 }
 
-- 
1.9.0

[PATCH 02/10] kill: fix coding style

[Sami Kerola]

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

Re: [PATCH 02/10] kill: fix coding style

[Karel Zak]

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

Re: [PATCH 02/10] kill: fix coding style

[Sami Kerola]

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/

[PATCH 03/10] kill: flip all comparions to be in smaller - greater order

[Sami Kerola]

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

Re: [PATCH 03/10] kill: flip all comparions to be in smaller - greater order

[Benno Schulenberg]

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

Re: [PATCH 03/10] kill: flip all comparions to be in smaller - greater order

[Sami Kerola]

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/

Re: [PATCH 03/10] kill: flip all comparions to be in smaller - greater order

[Benno Schulenberg]

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

[PATCH 04/10] kill: move magic numbers in beginning of the file

[Sami Kerola]

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

[PATCH 05/10] kill: make usage() not to return

[Sami Kerola]

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

Re: [PATCH 05/10] kill: make usage() not to return

[Benno Schulenberg]

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

Re: [PATCH 05/10] kill: make usage() not to return

[Sami Kerola]

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/

[PATCH 06/10] kill: leeway deprecation of --pid invocation as command name

[Sami Kerola]

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

Re: [PATCH 06/10] kill: leeway deprecation of --pid invocation as command name

[Bernhard Voelker]

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

Re: [PATCH 06/10] kill: leeway deprecation of --pid invocation as command name

[Sami Kerola]

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/

Re: [PATCH 06/10] kill: leeway deprecation of --pid invocation as command name

[Karel Zak]

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

[PATCH 07/10] kill: use control structure to pass user input to functions

[Sami Kerola]

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

Re: [PATCH 07/10] kill: use control structure to pass user input to functions

[Karel Zak]

On Sun, Mar 09, 2014 at 03:30:20PM -0500, Sami Kerola wrote:
> -			if (!check_all)
> +			if (ctl.check_all)
>  				proc_processes_filter_by_uid(ps, getuid());

 It would be nice to have regression test, right? :-)

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

[PATCH 08/10] kill: tell what is wrong rather than output usage()

[Sami Kerola]

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

[PATCH 09/10] kill: add parse_arguments() function

[Sami Kerola]

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

[PATCH 10/10] kill: move sigqueue inputs to control struct

[Sami Kerola]

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

Re: [PATCH 01/10] findfs: use symbolic exit values, and tell about them in manual

[Karel Zak]

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

Re: [PATCH 01/10] findfs: use symbolic exit values, and tell about them in manual

[Sami Kerola]

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

Re: [PATCH 01/10] findfs: use symbolic exit values, and tell about them in manual

[Karel Zak]

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