From owner-svn-src-all@FreeBSD.ORG Tue Oct 11 22:13:01 2011 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id E73971065674; Tue, 11 Oct 2011 22:13:01 +0000 (UTC) (envelope-from des@des.no) Received: from smtp.des.no (smtp.des.no [194.63.250.102]) by mx1.freebsd.org (Postfix) with ESMTP id 5EB738FC0C; Tue, 11 Oct 2011 22:13:01 +0000 (UTC) Received: from ds4.des.no (des.no [84.49.246.2]) by smtp.des.no (Postfix) with ESMTP id 55A521FFC33; Tue, 11 Oct 2011 22:12:57 +0000 (UTC) Received: by ds4.des.no (Postfix, from userid 1001) id 96C96B95F; Wed, 12 Oct 2011 00:12:57 +0200 (CEST) From: =?utf-8?Q?Dag-Erling_Sm=C3=B8rgrav?= To: Garrett Cooper References: <201110081247.p98Cl06s063337@svn.freebsd.org> Date: Wed, 12 Oct 2011 00:12:57 +0200 In-Reply-To: (Garrett Cooper's message of "Tue, 11 Oct 2011 08:45:24 -0700") Message-ID: <86wrcbrxcm.fsf@ds4.des.no> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.3 (berkeley-unix) MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r226157 - head/usr.bin/kdump X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 11 Oct 2011 22:13:02 -0000 --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Would you mind reviewing the attached patch, which cleans up ktrace(1)? DES --=20 Dag-Erling Sm=C3=B8rgrav - des@des.no --=-=-= Content-Type: text/x-patch Content-Disposition: inline; filename=ktrace-warns-6.diff Index: usr.bin/ktrace/ktrace.c =================================================================== --- usr.bin/ktrace/ktrace.c (revision 226262) +++ usr.bin/ktrace/ktrace.c (working copy) @@ -43,14 +43,15 @@ __FBSDID("$FreeBSD$"); #include +#include #include -#include #include -#include #include #include #include +#include +#include #include #include #include @@ -59,21 +60,22 @@ static char def_tracefile[] = DEF_TRACEFILE; +static enum clear { NOTSET, CLEAR, CLEARALL } clear = NOTSET; +static int pid; + static void no_ktrace(int); -static int rpid(char *); +static void set_pid_clear(const char *, enum clear); static void usage(void); int main(int argc, char *argv[]) { - enum { NOTSET, CLEAR, CLEARALL } clear; - int append, ch, fd, inherit, ops, pid, pidset, trpoints; + int append, ch, fd, inherit, ops, trpoints; const char *tracefile; mode_t omask; struct stat sb; - clear = NOTSET; - append = ops = pidset = inherit = 0; + append = ops = inherit = 0; trpoints = DEF_POINTS; tracefile = def_tracefile; while ((ch = getopt(argc,argv,"aCcdf:g:ip:t:")) != -1) @@ -82,11 +84,10 @@ append = 1; break; case 'C': - clear = CLEARALL; - pidset = 1; + set_pid_clear("1", CLEARALL); break; case 'c': - clear = CLEAR; + set_pid_clear(NULL, CLEAR); break; case 'd': ops |= KTRFLAG_DESCEND; @@ -95,15 +96,14 @@ tracefile = optarg; break; case 'g': - pid = -rpid(optarg); - pidset = 1; + set_pid_clear(optarg, NOTSET); + pid = -pid; break; case 'i': inherit = 1; break; case 'p': - pid = rpid(optarg); - pidset = 1; + set_pid_clear(optarg, NOTSET); break; case 't': trpoints = getpoints(optarg); @@ -115,12 +115,19 @@ default: usage(); } + argv += optind; argc -= optind; - - if ((pidset && *argv) || (!pidset && clear == NOTSET && !*argv)) + + /* must have either -[Cc], a pid or a command */ + if (clear == NOTSET && pid == 0 && argc == 0) usage(); - + /* can't have both a pid and a command */ + /* (note that -C sets pid to 1) */ + if (pid != 0 && argc > 0) { + usage(); + } + if (inherit) trpoints |= KTRFAC_INHERIT; @@ -129,10 +136,9 @@ if (clear == CLEARALL) { ops = KTROP_CLEAR | KTRFLAG_DESCEND; trpoints = ALL_POINTS; - pid = 1; - } else - ops |= pidset ? KTROP_CLEAR : KTROP_CLEARFILE; - + } else { + ops |= pid ? KTROP_CLEAR : KTROP_CLEARFILE; + } if (ktrace(tracefile, ops, trpoints, pid) < 0) err(1, "%s", tracefile); exit(0); @@ -160,46 +166,75 @@ trpoints |= PROC_ABI_POINTS; - if (*argv) { + if (argc > 0) { if (ktrace(tracefile, ops, trpoints, getpid()) < 0) err(1, "%s", tracefile); - execvp(argv[0], &argv[0]); - err(1, "exec of '%s' failed", argv[0]); + execvp(*argv, argv); + err(1, "exec of '%s' failed", *argv); } - else if (ktrace(tracefile, ops, trpoints, pid) < 0) + if (ktrace(tracefile, ops, trpoints, pid) < 0) err(1, "%s", tracefile); exit(0); } -static int -rpid(char *p) +static void +set_pid_clear(const char *p, enum clear cl) { - static int first; + intmax_t n; + char *e; - if (first++) { - warnx("only one -g or -p flag is permitted"); + if (clear != NOTSET && cl != NOTSET) { + /* either -c and -C or either of them twice */ + warnx("only one -c or -C flag is permitted"); usage(); } - if (!*p) { - warnx("illegal process id"); + if ((clear == CLEARALL && p != NULL) || (cl == CLEARALL && pid != 0)) { + /* both -C and a pid or pgid */ + warnx("the -C flag may not be combined with -g or -p"); usage(); } - return(atoi(p)); + if (p != NULL && pid != 0) { + /* either -p and -g or either of them twice */ + warnx("only one -g or -p flag is permitted"); + usage(); + } + if (p != NULL) { + errno = 0; + n = strtoimax(p, &e, 10); + /* + * 1) not a number, or outside the range of an intmax_t + * 2) inside the range of intmax_t but outside the range + * of an int, keeping in mind that the pid may be + * negated if it's actually a pgid. + */ + if (*e != '\0' || n < 1 || errno == ERANGE || + n > (intmax_t)INT_MAX || n > -(intmax_t)INT_MIN) { + warnx("invalid process or group id"); + usage(); + } + pid = n; + } + if (cl != NOTSET) + if ((clear = cl) == CLEARALL) + pid = 1; } static void usage(void) { - (void)fprintf(stderr, "%s\n%s\n", -"usage: ktrace [-aCcdi] [-f trfile] [-g pgrp | -p pid] [-t trstr]", -" ktrace [-adi] [-f trfile] [-t trstr] command"); + + fprintf(stderr, "%s\n%s\n", + "usage: ktrace [-aCcdi] [-f trfile] [-g pgrp | -p pid] [-t trstr]", + " ktrace [-adi] [-f trfile] [-t trstr] command"); exit(1); } static void no_ktrace(int sig __unused) { - (void)fprintf(stderr, -"error:\tktrace() system call not supported in the running kernel\n\tre-compile kernel with 'options KTRACE'\n"); + + fprintf(stderr, "error:\t%s\n\t%s\n", + "ktrace() system call not supported in the running kernel", + "re-compile kernel with 'options KTRACE'"); exit(1); } Index: usr.bin/ktrace/Makefile =================================================================== --- usr.bin/ktrace/Makefile (revision 226262) +++ usr.bin/ktrace/Makefile (working copy) @@ -5,6 +5,4 @@ SRCS= ktrace.c subr.c MLINKS= ktrace.1 trace.1 -WARNS?= 4 - .include --=-=-=--