From owner-svn-src-head@freebsd.org Tue Aug 4 05:33:14 2015 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id DBFD29B20E4; Tue, 4 Aug 2015 05:33:14 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail108.syd.optusnet.com.au (mail108.syd.optusnet.com.au [211.29.132.59]) by mx1.freebsd.org (Postfix) with ESMTP id 886FF19B6; Tue, 4 Aug 2015 05:33:14 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from c211-30-166-197.carlnfd1.nsw.optusnet.com.au (c211-30-166-197.carlnfd1.nsw.optusnet.com.au [211.30.166.197]) by mail108.syd.optusnet.com.au (Postfix) with ESMTPS id 8855D1A248B; Tue, 4 Aug 2015 15:33:05 +1000 (AEST) Date: Tue, 4 Aug 2015 15:33:04 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Marcelo Araujo cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r286267 - head/usr.bin/ypcat In-Reply-To: <201508040241.t742fFDi048172@repo.freebsd.org> Message-ID: <20150804150500.C896@besplex.bde.org> References: <201508040241.t742fFDi048172@repo.freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.1 cv=ItbjC+Lg c=1 sm=1 tr=0 a=KA6XNC2GZCFrdESI5ZmdjQ==:117 a=PO7r1zJSAAAA:8 a=JzwRw_2MAAAA:8 a=kj9zAlcOel0A:10 a=CgiagczvAAAA:8 a=DayoRurDc5bhoHBYV1gA:9 a=CjuIK1q_8ugA:10 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 04 Aug 2015 05:33:15 -0000 On Tue, 4 Aug 2015, Marcelo Araujo wrote: > Log: > Remove the 3rd clause of BSD LICENSE. > Sync the code with the OpenBSD version. > Small style(9) fix up. This has many style(9) fix downs. > Differential Revision: D3212 > Reviewed by: rodrigc, bapt > Obtained from: OpenBSD > Sponsored by: gandi.net > > Modified: > head/usr.bin/ypcat/ypcat.c > > Modified: head/usr.bin/ypcat/ypcat.c > ============================================================================== > --- head/usr.bin/ypcat/ypcat.c Tue Aug 4 02:34:51 2015 (r286266) > +++ head/usr.bin/ypcat/ypcat.c Tue Aug 4 02:41:14 2015 (r286267) > @@ -34,18 +33,20 @@ __FBSDID("$FreeBSD$"); > #include > #include > #include > +#include > +#include > +#include > +#include > +#include > +#include > > #include > #include > -#include > +#include > #include > > -#include > -#include > -#include > -#include > -#include > -#include Unsorting. The old organization was almost perfect except for including both sys/types and sys/param.h. Now there isn't even a blank line separating one pair of the 3 groupes of headers. > +void usage(void); > +int printit(u_long, char *, int, char *, int, void *); Unsorted declarations. Lost staticization. usage() was misplaced early in the file, so that since it was static, it didn't need a forward declaration, except style(9) requires one. usage() is its main example. > > static const struct ypalias { > char *alias, *name; > @@ -64,17 +65,18 @@ static const struct ypalias { > > static int key; > > -static void > +void Lost staticization. > usage(void) > { > - fprintf(stderr, "%s\n%s\n", > - "usage: ypcat [-kt] [-d domainname] mapname", > - " ypcat -x"); > + fprintf(stderr, > + "usage: ypcat [-kt] [-d domainname] mapname\n" > + " ypcat -x\n"); This fixes the indentation, but breaks the string using C90 string concatenation. style(9) doesn't specify using the "%s\n%s\n..." for printing multiple strings on multiple line by example, but almost all FreeBSD utilities including the old version of this one provide an example of this. > exit(1); > } > > -static int > -printit(unsigned long instatus, char *inkey, int inkeylen, char *inval, int invallen, void *dummy __unused) > +int > +printit(u_long instatus, char *inkey, int inkeylen, char *inval, int invallen, > + void *indata) Lost staticization. > { > if (instatus != YP_TRUE) > return (instatus); > @@ -87,31 +89,29 @@ printit(unsigned long instatus, char *in > int > main(int argc, char *argv[]) > { > - char *domainname = NULL; > + char *domain = NULL, *inmap; > struct ypall_callback ypcb; > - char *inmap; > - int notrans; > - int c, r; > + extern char *optarg; > + extern int optind; Extern declarations belong in a header file. Broken programs sometimes declare them directly to hide the bug that they don't include the correct header file, but this program still includes the correct header file (unistd.h>). Compilers are directed to warn about nested externs at a not very high WARNS. > + int notrans, c, r; Combining the declaration is good, but the variables are still unsorted. > u_int i; > > notrans = key = 0; > - > while ((c = getopt(argc, argv, "xd:kt")) != -1) > switch (c) { > case 'x': > - for (i = 0; i + for (i=0; i ... > @@ -120,24 +120,29 @@ main(int argc, char *argv[]) > if (optind + 1 != argc) > usage(); > > - if (!domainname) > - yp_get_default_domain(&domainname); > + if (!domain) > + yp_get_default_domain(&domain); Still uses '!' on pointers. > > inmap = argv[optind]; > - for (i = 0; (!notrans) && i - if (strcmp(inmap, ypaliases[i].alias) == 0) > - inmap = ypaliases[i].name; > + if (!notrans) { > + for (i=0; i case YPERR_YPBIND: > - errx(1, "not running ypbind"); > + errx(1, "ypcat: not running ypbind\n"); > + exit(1); Large regressions. Now the program name is printed twice, and the newline at the end of the message is printed twice, and there are 2 exits, with the new one unreachable. > default: > - errx(1, "no such map %s. reason: %s", inmap, yperr_string(r)); > + errx(1, "No such map %s. Reason: %s\n", > + inmap, yperr_string(r)); > + exit(1); As above, except the program name is not printed twice, plus - the first word after the program name is now capitalized. This is part of a sentence beginning with the program name and a colon. There are various style guides for capitilization after a colon. FreeBSD uses the following inconsistent rules in error messages: - don't capitalize - however, if the error message is from strerror(), then it capitalizes. Keep this. - the spacing and phrasing for the second sentence is not so good and is not improved by capitalization. It is more normal to use further punctuation in the same sentence. > } > exit(0); > } Bruce