Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 10 Jan 2002 23:49:36 +0200 (EET)
From:      Andrey Simonenko <simon@simon.org.ua>
To:        FreeBSD-gnats-submit@freebsd.org
Subject:   bin/33774: Patch for killall(1)
Message-ID:  <20020110234631.Q8106-100000@lion.com.ua>

next in thread | raw e-mail | index | archive | help

>Number:         33774
>Category:       bin
>Synopsis:       Patch for killall(1)
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Thu Jan 10 13:50:00 PST 2002
>Closed-Date:
>Last-Modified:
>Originator:     Andrey Simonenko
>Release:        FreeBSD 4.4-STABLE i386
>Organization:
>Environment:

System: FreeBSD 4.4-STABLE i386

killall.c CVS version:
$FreeBSD: src/usr.bin/killall/killall.c,v 1.5.2.4 2001/05/19 19:22:49 phk Exp $

>Description:

Following patch fixes some bugs, speeds up killall(1).

Bugs and parts of killall(1) which have been fixed:

1. Sometime it is impossible to use signal names without "sig" prefix and
written in lower case. For example command "killall -term smth" tries
to stat "/dev/ttyerm" device and command "killall -usr1 smth" tries to
find user "sr1". According to killall source (line #186) case insensetive
signal names are supported.

2. It is not checked if there is an argument for the -t, -u and -c options.
Command line arguments parsing was re-written with getopt(3) function. So,
now it is possible to use "-usr1" for the SIGUSR1 and "-u sr1" for the
"sr1" user name.

3. It is possible to send signal #32 to a process (line #196), 32 is the value
of the NSIG macro. Really last signal has number #31, that is NSIG-1.

4. There is some sense to use killall(1) program if at least one process
name will be passed to it. It is possible to run killall(1) with some
arguments, and killall(1) will do nothing (line #208).

5. If the -u option is not used and there isn't entry in the /etc/passwd file
for the user, who run killall(1), then KERN_PROC_RUID isn't used in sysctl()
call (lines #234-238 and #249). killall(1) can be run not only but some user,
who logon on system. Also it was removed comparision for the mib[2] in the
#249 line, because it is always true.

6. Speedups to lines #265-271 code. "*newprocs" variable has been removed.

7. If the -m flag is used, then all RE are compiled for every process got
from the sysctl() call. So, I removed regexec() and regfree() in the loop
(line #286). Instead dynamic array is allocated and all RE are compiled only
once and if there is a problem with some RE, then an error message with the
problem description is printed.

8. I didn't find sense of using "pmatch" variable, so I removed it.

9. Variable "matched" was removed, because whole loop (line #286) was
simplified.

10. It's possible to specify incorect RE with the -m flag and in this case
killall(1) "ignores" the -m flag.

11. nosig() function was removed, becase of using getopt() function and
more accurate command line argument parsing.

12. usage() function was fixed, so now it displays correct arguments list.

Bugs and feautures which require clarification:

1. I didn't find any good reson for the -c option, its implementation in the
killall(1) source doesn't look like what is described in the killall(1)
manual page. So, something (source or man-page) should be fixed.

2. Manual page says that the -m option makes arguments case insensitive RE.
In the source arguments are case sensitive RE. So, something should be fixed.


Comments?


>How-To-Repeat:

>Fix:
--- /usr/src/usr.bin/killall/killall.c	Sun May 20 08:06:32 2001
+++ killall.c	Thu Jan 10 22:53:26 2002
@@ -30,6 +30,7 @@
 #include <sys/cdefs.h>
 #include <sys/param.h>
 #include <sys/stat.h>
+#include <sys/types.h>
 #include <sys/user.h>
 #include <sys/sysctl.h>
 #include <fcntl.h>
@@ -43,6 +44,8 @@
 #include <ctype.h>
 #include <err.h>
 #include <errno.h>
+#include <libgen.h>
+#include <limits.h>
 #include <unistd.h>

 static char	*prog;
@@ -50,8 +53,7 @@
 static void __dead2
 usage(void)
 {
-
-	fprintf(stderr, "usage: %s [-l] [-v] [-m] [-sig] [-u user] [-t tty] [-c cmd] [cmd]...\n", prog);
+	fprintf(stderr, "usage: %s [-help] [-?hlvdms] [-u user] [-t tty] [-signal] [-c procname] [procname ...]\n", prog);
 	fprintf(stderr, "At least one option or argument to specify processes must be given.\n");
 	exit(1);
 }
@@ -62,8 +64,7 @@
 	static char buf[80];
 	char *s;

-	strncpy(buf, str, sizeof(buf));
-	buf[sizeof(buf) - 1] = '\0';
+	strlcpy(buf, str, sizeof(buf));
 	for (s = buf; *s; s++)
 		*s = toupper(*s);
 	return buf;
@@ -87,24 +88,15 @@
 	fprintf(fp, "\n");
 }

-static void
-nosig(char *name)
-{
-
-	warnx("unknown signal %s; valid signals:", name);
-	printsig(stderr);
-	exit(1);
-}
-
 int
 main(int ac, char **av)
 {
-	struct kinfo_proc *procs = NULL, *newprocs;
+	struct kinfo_proc *procs = NULL;
 	struct stat	sb;
 	struct passwd	*pw;
-	regex_t		rgx;
-	regmatch_t	pmatch;
+	regex_t		*rgx = NULL;
 	int		i, j;
+	char		*avopt;
 	char		buf[256];
 	char		*user = NULL;
 	char		*tty = NULL;
@@ -119,7 +111,7 @@
 	pid_t		thispid;
 	uid_t		thisuid;
 	dev_t		thistdev;
-	int		sig = SIGTERM;
+	int		sig = SIGTERM, sigtmp;
 	const char *const *p;
 	char		*ep;
 	int		errors = 0;
@@ -127,85 +119,83 @@
 	size_t		miblen;
 	int		st, nprocs;
 	size_t		size;
-	int		matched;
 	int		killed = 0;
+	int		opt;

-	prog = av[0];
-	av++;
-	ac--;
-
-	while (ac > 0) {
-		if (strcmp(*av, "-l") == 0) {
-			printsig(stdout);
-			exit(0);
-		}
-		if (strcmp(*av, "-help") == 0)
+	prog = basename(av[0]);
+	opterr = 0;
+	for (; optind < ac;) {
+		avopt = av[optind];
+		if (*avopt != '-')
+			break;
+		++avopt;
+		if (*avopt == '?' || strcmp(avopt, "help") == 0)
 			usage();
-		if (**av == '-') {
-			++*av;
-			switch (**av) {
-			case 'u':
-				++*av;
-				if (**av == '\0')
-					++av;
-				--ac;
-				user = *av;
-				break;
-			case 't':
-				++*av;
-				if (**av == '\0')
-					++av;
-				--ac;
-				tty = *av;
-				break;
-			case 'c':
-				++*av;
-				if (**av == '\0')
-					++av;
-				--ac;
-				cmd = *av;
-				break;
-			case 'v':
-				vflag++;
-				break;
-			case 's':
-				sflag++;
-				break;
-			case 'd':
-				dflag++;
-				break;
-			case 'm':
-				mflag++;
-				break;
-			default:
-				if (isalpha(**av)) {
-					if (strncasecmp(*av, "sig", 3) == 0)
-						*av += 3;
-					for (sig = NSIG, p = sys_signame + 1;
-					     --sig; ++p)
-						if (strcasecmp(*p, *av) == 0) {
-							sig = p - sys_signame;
-							break;
-						}
-					if (!sig)
-						nosig(*av);
-				} else if (isdigit(**av)) {
-					sig = strtol(*av, &ep, 10);
-					if (!*av || *ep)
-						errx(1, "illegal signal number: %s", *av);
-					if (sig < 0 || sig > NSIG)
-						nosig(*av);
-				} else
-					nosig(*av);
+		if (isalpha(*avopt)) {
+			if (strncasecmp(avopt, "sig", 3) == 0)
+				avopt += 3;
+			for (sigtmp = NSIG, p = sys_signame + 1; --sigtmp; ++p)
+				if (strcasecmp(*p, avopt) == 0) {
+					sigtmp = p - sys_signame;
+					break;
+				}
+			if (sigtmp != 0) {
+				++optind;
+				sig = sigtmp;
+				continue;
+			}
+		} else if (isdigit(*avopt)) {
+			sigtmp = strtol(avopt, &ep, 10);
+			if (*ep != '\0')
+				errx(1, "illegal signal number: %s", avopt);
+			if (sigtmp > 0 && sigtmp < NSIG) {
+				++optind;
+				sig = sigtmp;
+				continue;
 			}
-			++av;
-			--ac;
-		} else {
+			errx(1, "incorrect signal number: %d", sigtmp);
+		}
+		if ( (opt = getopt(ac, av, "dvhlmsu:t:c:")) == -1)
+			break;
+		switch (opt) {
+		case 'u':
+			user = optarg;
+			break;
+		case 't':
+			tty = optarg;
+			break;
+		case 'c':
+			cmd = optarg;
+			break;
+		case 'v':
+			vflag = 1;
+			break;
+		case 's':
+			sflag = 1;
 			break;
+		case 'd':
+			dflag = 1;
+			break;
+		case 'm':
+			mflag = 1;
+			break;
+		case 'l':
+			printsig(stdout);
+			return 0;
+		case 'h':
+			usage();
+		case '?':
+		default:
+			warnx("invalid switch -%c or unknown signal; valid signals:", optopt);
+			printsig(stderr);
+			return 1;
 		}
 	}

-	if (user == NULL && tty == NULL && cmd == NULL && ac == 0)
+	ac -= optind;
+	av += optind;
+
+	if (ac == 0)
 		usage();

 	if (tty) {
@@ -228,18 +218,25 @@
 		if (pw == NULL)
 			errx(1, "user %s does not exist", user);
 		uid = pw->pw_uid;
-		if (dflag)
-			printf("uid:%d\n", uid);
-	} else {
+		endpwent();
+	} else
 		uid = getuid();
-		if (uid != 0) {
-			pw = getpwuid(uid);
-			if (pw)
-				user = pw->pw_name;
-			if (dflag)
-				printf("uid:%d\n", uid);
-		}
+
+	if (mflag) {
+		if ( (rgx = malloc((ac + 1) * sizeof *rgx)) == NULL)
+			err(1, "could not allocate memory");
+		if (cmd != NULL)
+			if ( (i = regcomp(&rgx[ac], cmd, REG_EXTENDED|REG_NOSUB)) != 0) {
+				regerror(i, (regex_t *)NULL, buf, sizeof buf);
+				errx(1, "%s: illegal regexp: %s", cmd, buf);
+			}
+		for (j = 0; j < ac; j++)
+			if ( (i = regcomp(&rgx[j], av[j], REG_EXTENDED|REG_NOSUB)) != 0) {
+				regerror(i, (regex_t *)NULL, buf, sizeof buf);
+				errx(1, "%s: illegal regexp: %s", av[j], buf);
+			}
 	}
+
 	size = 0;
 	mib[0] = CTL_KERN;
 	mib[1] = KERN_PROC;
@@ -247,7 +244,7 @@
 	mib[3] = 0;
 	miblen = 3;

-	if (user && mib[2] == KERN_PROC_ALL) {
+	if (user != NULL || uid != 0) {
 		mib[2] = KERN_PROC_RUID;
 		mib[3] = uid;
 		miblen = 4;
@@ -261,26 +258,21 @@
 	st = sysctl(mib, miblen, NULL, &size, NULL, 0);
 	do {
 		size += size / 10;
-		newprocs = realloc(procs, size);
-		if (newprocs == 0) {
-			if (procs)
-				free(procs);
-			errx(1, "could not reallocate memory");
-		}
-		procs = newprocs;
+		if ( (procs = realloc(procs, size)) == NULL)
+			err(1, "could not reallocate memory");
 		st = sysctl(mib, miblen, procs, &size, NULL, 0);
 	} while (st == -1 && errno == ENOMEM);
 	if (st == -1)
 		err(1, "could not sysctl(KERN_PROC)");
 	if (size % sizeof(struct kinfo_proc) != 0) {
-		fprintf(stderr, "proc size mismatch (%d total, %d chunks)\n",
-			size, sizeof(struct kinfo_proc));
+		fprintf(stderr, "%s: proc size mismatch (%d total, %d chunks)\n",
+			prog, size, sizeof(struct kinfo_proc));
 		fprintf(stderr, "userland out of sync with kernel, recompile libkvm etc\n");
 		exit(1);
 	}
 	nprocs = size / sizeof(struct kinfo_proc);
 	if (dflag)
-		printf("nprocs %d\n", nprocs);
+		printf("uid:%d\nnprocs %d\n", uid, nprocs);

 	for (i = 0; i < nprocs; i++) {
 		thispid = procs[i].kp_proc.p_pid;
@@ -289,61 +281,33 @@
 		thistdev = procs[i].kp_eproc.e_tdev;
 		thisuid = procs[i].kp_eproc.e_pcred.p_ruid;	/* real uid */

-		matched = 1;
 		if (user) {
 			if (thisuid != uid)
-				matched = 0;
+				continue;
 		}
 		if (tty) {
 			if (thistdev != tdev)
-				matched = 0;
+				continue;
 		}
 		if (cmd) {
 			if (mflag) {
-				if (regcomp(&rgx, cmd,
-				    REG_EXTENDED|REG_NOSUB) != 0) {
-					mflag = 0;
-					warnx("%s: illegal regexp", cmd);
-				}
-			}
-			if (mflag) {
-				pmatch.rm_so = 0;
-				pmatch.rm_eo = strlen(thiscmd);
-				if (regexec(&rgx, thiscmd, 0, &pmatch,
-				    REG_STARTEND) != 0)
-					matched = 0;
-				regfree(&rgx);
+				if (regexec(&rgx[ac], thiscmd, 0, (regmatch_t *)0, 0) != 0)
+					continue;
 			} else {
 				if (strcmp(thiscmd, cmd) != 0)
-					matched = 0;
+					continue;
 			}
 		}
-		if (matched == 0)
-			continue;
-		matched = 0;
 		for (j = 0; j < ac; j++) {
 			if (mflag) {
-				if (regcomp(&rgx, av[j],
-				    REG_EXTENDED|REG_NOSUB) != 0) {
-					mflag = 0;
-					warnx("%s: illegal regexp", av[j]);
-				}
-			}
-			if (mflag) {
-				pmatch.rm_so = 0;
-				pmatch.rm_eo = strlen(thiscmd);
-				if (regexec(&rgx, thiscmd, 0, &pmatch,
-				    REG_STARTEND) == 0)
-					matched = 1;
-				regfree(&rgx);
+				if (regexec(&rgx[j], thiscmd, 0, (regmatch_t *)0, 0) == 0)
+					break;
 			} else {
 				if (strcmp(thiscmd, av[j]) == 0)
-					matched = 1;
+					break;
 			}
-			if (matched)
-				break;
 		}
-		if (matched == 0)
+		if (j == ac)
 			continue;
 		if (dflag)
 			printf("sig:%d, cmd:%s, pid:%d, dev:0x%x uid:%d\n", sig,
@@ -353,7 +317,7 @@
 			printf("kill -%s %d\n", upper(sys_signame[sig]),
 			    thispid);

-		killed++;
+		killed = 1;
 		if (!dflag && !sflag) {
 			if (kill(thispid, sig) < 0 /* && errno != ESRCH */ ) {
 				warn("kill -%s %d", upper(sys_signame[sig]),
@@ -367,5 +331,5 @@
 		    getuid() != 0 ? "belonging to you " : "");
 		errors = 1;
 	}
-	exit(errors);
+	return errors;
 }

>Release-Note:
>Audit-Trail:
>Unformatted:

To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-bugs" in the body of the message




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20020110234631.Q8106-100000>