From owner-svn-src-all@FreeBSD.ORG Wed Jan 18 23:10:26 2012 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 6642C106566B; Wed, 18 Jan 2012 23:10:26 +0000 (UTC) (envelope-from guy.helmer@palisadesystems.com) Received: from ps-1-a.compliancesafe.com (ps-1-a.compliancesafe.com [216.81.161.161]) by mx1.freebsd.org (Postfix) with ESMTP id C484D8FC13; Wed, 18 Jan 2012 23:10:25 +0000 (UTC) Received: from mail.palisadesystems.com (localhost [127.0.0.1]) by ps-1-a.compliancesafe.com (8.14.4/8.14.3) with ESMTP id q0INA79M012064; Wed, 18 Jan 2012 17:10:07 -0600 (CST) (envelope-from guy.helmer@palisadesystems.com) Received: from [192.168.221.36] ([192.168.221.36]) (authenticated bits=0) by mail.palisadesystems.com (8.14.3/8.14.3) with ESMTP id q0INA1u6024846 (version=TLSv1/SSLv3 cipher=AES128-SHA bits=128 verify=NO); Wed, 18 Jan 2012 17:10:03 -0600 (CST) (envelope-from guy.helmer@palisadesystems.com) X-DKIM: Sendmail DKIM Filter v2.8.3 mail.palisadesystems.com q0INA1u6024846 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=palisadesystems.com; s=mail; t=1326928204; bh=Vt7BMzfL1XtSpHJJldRFKr9m1ZRRcru6XQCAv13BBak=; l=128; h=Subject:Mime-Version:Content-Type:From:In-Reply-To:Date:Cc: Content-Transfer-Encoding:Message-Id:References:To; b=RE69+86CdD1PiuJp8Guqa3pL0X3YBId/m3EmaLbPVrZCX6hAKs8JGJ1eTf5KKyOCv wqYOZN1Q4cmW4ZEdjcT5KBrd/WmItPsNtcvL1oEKA5aotWUlKRxZ0AgFvbiG08LITo GhS00LKMD1WBAyBSUnVSIwy/d8uD/Ku2FUfNSE4w= Mime-Version: 1.0 (Apple Message framework v1251.1) Content-Type: text/plain; charset=us-ascii From: Guy Helmer In-Reply-To: <20120117201411.B1819@besplex.bde.org> Date: Wed, 18 Jan 2012 17:10:04 -0600 Content-Transfer-Encoding: quoted-printable Message-Id: <8DD4D210-28DA-453F-86D0-6FC4FB84AC04@palisadesystems.com> References: <201201122249.q0CMnaZe030200@svn.freebsd.org> <20120114204720.Q1458@besplex.bde.org> <20120114182758.GJ1694@garage.freebsd.pl> <20120115073823.O843@besplex.bde.org> <52A73054-9960-403B-B2FE-857C8801D129@palisadesystems.com> <20120116175627.GA1674@garage.freebsd.pl> <20120117201411.B1819@besplex.bde.org> To: Bruce Evans X-Mailer: Apple Mail (2.1251.1) X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.5 (mail.palisadesystems.com [172.16.1.5]); Wed, 18 Jan 2012 17:10:04 -0600 (CST) X-Palisade-MailScanner-Information: Please contact the ISP for more information X-Palisade-MailScanner-ID: q0INA1u6024846 X-Palisade-MailScanner: Found to be clean X-Palisade-MailScanner-SpamCheck: not spam, SpamAssassin (score=-1.628, required 5, ALL_TRUSTED -1.00, BAYES_00 -1.90, RP_8BIT 1.27) X-Palisade-MailScanner-From: guy.helmer@palisadesystems.com X-Spam-Status: No X-PacketSure-Scanned: Yes Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org, Pawel Jakub Dawidek Subject: Re: svn commit: r230037 - head/lib/libutil 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: Wed, 18 Jan 2012 23:10:26 -0000 On Jan 17, 2012, at 3:58 AM, Bruce Evans wrote: > On Mon, 16 Jan 2012, Pawel Jakub Dawidek wrote: >=20 >> On Mon, Jan 16, 2012 at 11:14:32AM -0600, Guy Helmer wrote: >>> I've pasted the diff below that I think captures the majority of the = issues you have brought up. I have not attempted to tackle the = property.3/properties.3 issues, nor the objections to the prefixes that = I think would take considerably more effort to resolve -- I wanted to = concentrate on the issues that can be isolated to libutil. I hope the = diff was pasted OK, especially WRT characters. >>=20 >> The patch looks mostly good, one nit mentioned below and also one >> question for Bruce. >>=20 >>> +/* Flags for hexdump(3) */ >>> +/* Flags for humanize_number(3) flags */ >>> +/* Flags for humanize_number(3) scale */ >>> +/* return values from realhostname() */ >>> +/* Flags for pw_scan() */ >>> +/* Return values from uu_lock() */ >>=20 >> All those sentences are missing period and one doesn't start with >> capital letter. >=20 > I decided not to worry about the termination since it was fairly = consistent > in the file. The most recent commit fixed these, but made all the = others > inconsistent: >=20 > % /* for properties.c */ > % /* Avoid pulling in all the include files for no need */ > % #ifdef _STDIO_H_ /* avoid adding new includes */ >=20 > This is now the only comment to the right of code. Comments to the > right of ifdefs are especially hard format nicely > (the normal comment indentation to 32 or 40 columns doesn't work > well; I normally use a single space; the above indents to 24 = columns), > so the should be avoided. The same treatment is used for 3 other = includes, > but there is no comment for the others. The simplest fix is to remove > this comment. Otherwise, put a meta-comment about them all somewhere. > It's hard to place this so that it clearly covers them all, but = anywhere > is better than to the right of 1 of their ifdefs. >=20 > % /* fparseln(3) */ >=20 > This is also missing the new, otherwise (too) uniform wording > "Flags for..." >=20 >> I noticed also one more inconsistency: >>=20 >> struct kinfo_file * >> kinfo_getfile(pid_t _pid, int *_cntp); >> struct passwd >> *pw_dup(const struct passwd *_pw); >>=20 >> Sometimes * is on the same line as function type and sometimes it is = in >> the line below. Former is definiately better. >=20 > Indded. >=20 OK, one more round of proposed changes: Index: lib/libutil/libutil.h =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- lib/libutil/libutil.h (revision 230318) +++ lib/libutil/libutil.h (working copy) @@ -71,14 +71,14 @@ #define PROPERTY_MAX_NAME 64 #define PROPERTY_MAX_VALUE 512 =20 -/* for properties.c */ +/* For properties.c. */ typedef struct _property { struct _property *next; char *name; char *value; } *properties; =20 -/* Avoid pulling in all the include files for no need */ +/* Avoid pulling in all the include files for no need. */ struct in_addr; struct pidfh; struct sockaddr; @@ -132,7 +132,11 @@ int uu_unlock(const char *_ttyname); int uu_lock_txfr(const char *_ttyname, pid_t _pid); =20 -#ifdef _STDIO_H_ /* avoid adding new includes */ +/* + * Conditionally prototype the following functions if the include + * files upon which they depend have been included. + */ +#ifdef _STDIO_H_ char *fparseln(FILE *_fp, size_t *_len, size_t *_lineno, const char _delim[3], int _flags); #endif @@ -150,26 +154,26 @@ char *pw_make_v7(const struct passwd *_pw); int pw_mkdb(const char *_user); int pw_lock(void); -struct passwd - *pw_scan(const char *_line, int _flags); -const char - *pw_tempname(void); +struct passwd * + pw_scan(const char *_line, int _flags); +const char * + pw_tempname(void); int pw_tmp(int _mfd); #endif =20 #ifdef _GRP_H_ int gr_copy(int __ffd, int _tfd, const struct group *_gr, struct group *_old_gr); -struct group - *gr_dup(const struct group *_gr); +struct group * + gr_dup(const struct group *_gr); int gr_equal(const struct group *_gr1, const struct group *_gr2); void gr_fini(void); int gr_init(const char *_dir, const char *_master); int gr_lock(void); char *gr_make(const struct group *_gr); int gr_mkdb(void); -struct group - *gr_scan(const char *_line); +struct group * + gr_scan(const char *_line); int gr_tmp(int _mdf); #endif =20 @@ -209,18 +213,18 @@ #define HD_OMIT_HEX (1 << 17) #define HD_OMIT_CHARS (1 << 18) =20 -/* Flags for humanize_number(3) flags. */ +/* Values for humanize_number(3)'s flags parameter. */ #define HN_DECIMAL 0x01 #define HN_NOSPACE 0x02 #define HN_B 0x04 #define HN_DIVISOR_1000 0x08 #define HN_IEC_PREFIXES 0x10 =20 -/* Flags for humanize_number(3) scale. */ +/* Values for humanize_number(3)'s scale parameter. */ #define HN_GETSCALE 0x10 #define HN_AUTOSCALE 0x20 =20 -/* return values from realhostname(). */ +/* Return values from realhostname(). */ #define HOSTNAME_FOUND 0 #define HOSTNAME_INCORRECTNAME 1 #define HOSTNAME_INVALIDADDR 2 @@ -233,12 +237,12 @@ /* Return values from uu_lock(). */ #define UU_LOCK_INUSE 1 #define UU_LOCK_OK 0 -#define UU_LOCK_OPEN_ERR -1 -#define UU_LOCK_READ_ERR -2 -#define UU_LOCK_CREAT_ERR -3 -#define UU_LOCK_WRITE_ERR -4 -#define UU_LOCK_LINK_ERR -5 -#define UU_LOCK_TRY_ERR -6 -#define UU_LOCK_OWNER_ERR -7 +#define UU_LOCK_OPEN_ERR (-1) +#define UU_LOCK_READ_ERR (-2) +#define UU_LOCK_CREAT_ERR (-3) +#define UU_LOCK_WRITE_ERR (-4) +#define UU_LOCK_LINK_ERR (-5) +#define UU_LOCK_TRY_ERR (-6) +#define UU_LOCK_OWNER_ERR (-7) =20 #endif /* !_LIBUTIL_H_ */ -------- This message has been scanned by ComplianceSafe, powered by Palisade's PacketSure.