From owner-svn-src-head@freebsd.org Thu Jun 25 03:24: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 2024A98BEC8; Thu, 25 Jun 2015 03:24:14 +0000 (UTC) (envelope-from bdrewery@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:1900:2254:206c::16:87]) by mx1.freebsd.org (Postfix) with ESMTP id 06BA62CB9; Thu, 25 Jun 2015 03:24:14 +0000 (UTC) (envelope-from bdrewery@FreeBSD.org) Received: from mail.xzibition.com (localhost [IPv6:::1]) by freefall.freebsd.org (Postfix) with ESMTP id EF80D1BC8; Thu, 25 Jun 2015 03:24:13 +0000 (UTC) (envelope-from bdrewery@FreeBSD.org) Received: from mail.xzibition.com (localhost [172.31.3.2]) by mail.xzibition.com (Postfix) with ESMTP id 8A4EDA54F; Thu, 25 Jun 2015 03:24:13 +0000 (UTC) X-Virus-Scanned: amavisd-new at mail.xzibition.com Received: from mail.xzibition.com ([172.31.3.2]) by mail.xzibition.com (mail.xzibition.com [172.31.3.2]) (amavisd-new, port 10026) with LMTP id oKARoaKsBlcF; Thu, 25 Jun 2015 03:24:10 +0000 (UTC) Subject: Re: svn commit: r284163 - head/bin/cp DKIM-Filter: OpenDKIM Filter v2.9.2 mail.xzibition.com E9727A549 To: Bruce Evans References: <201506081924.t58JOJQw095752@svn.freebsd.org> <20150609152946.Y935@besplex.bde.org> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org From: Bryan Drewery Openpgp: id=F9173CB2C3AAEA7A5C8A1F0935D771BB6E4697CF; url=http://www.shatow.net/bryan/bryan2.asc X-Enigmail-Draft-Status: N1110 Organization: FreeBSD Message-ID: <558B745C.8040305@FreeBSD.org> Date: Wed, 24 Jun 2015 22:24:12 -0500 User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.0.1 MIME-Version: 1.0 In-Reply-To: <20150609152946.Y935@besplex.bde.org> Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="5TnR9hbiIaAbEsS7FB5tPK1o5mXoQammU" 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: Thu, 25 Jun 2015 03:24:14 -0000 This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --5TnR9hbiIaAbEsS7FB5tPK1o5mXoQammU Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable On 6/9/2015 1:28 AM, Bruce Evans wrote: > On Mon, 8 Jun 2015, Bryan Drewery wrote: >=20 >> Log: >> Cleanup some style(9) issues. >> >> - Whitespace. >> - Comments. >> - Wrap long lines. >=20 > cp's style had a remarlable amount of bitrot. >=20 > This change unimproves it in some places. I have been traveling and packing. I'm replying now but won't have time to address the issues until next week. I was trying to avoid doing any of this but touched code which was horrendously misstyled and chained into reindenting the whole file and doing it wrong :). At this point I don't want to tweak this much more. >> --- head/bin/cp/cp.c Mon Jun 8 19:13:04 2015 (r284162) >> +++ head/bin/cp/cp.c Mon Jun 8 19:24:18 2015 (r284163) >> @@ -75,8 +75,8 @@ __FBSDID("$FreeBSD$"); >> #include "extern.h" >> >> #define STRIP_TRAILING_SLASH(p) { \ >> - while ((p).p_end > (p).p_path + 1 && (p).p_end[-1] =3D=3D '/'= ) \ >> - *--(p).p_end =3D 0; \ >> + while ((p).p_end > (p).p_path + 1 && (p).p_end[-1] =3D=3D '/') = \ >> + *--(p).p_end =3D 0; \ >=20 Woops. >=20 >> @@ -245,10 +245,10 @@ main(int argc, char *argv[]) >> type =3D FILE_TO_FILE; >> >> if (have_trailing_slash && type =3D=3D FILE_TO_FILE) { >> - if (r =3D=3D -1) >> + if (r =3D=3D -1) { >=20 > This adds excessive braces. >=20 >> errx(1, "directory %s does not exist", >> - to.p_path); >> - else >> + to.p_path); >> + } else It is a multi-line statement due to the hard 80-width wrap. I feel it is fine in this case. >> errx(1, "%s is not a directory", to.p_path); >> } >> } else >> ... >> @@ -379,7 +379,8 @@ copy(char *argv[], enum op type, int fts >> mode =3D curr->fts_statp->st_mode; >> if ((mode & (S_ISUID | S_ISGID | S_ISTXT)) || >> ((mode | S_IRWXU) & mask) !=3D (mode & mask)) >> - if (chmod(to.p_path, mode & mask) !=3D 0){ >> + if (chmod(to.p_path, mode & mask) !=3D >> + 0) { >> warn("chmod: %s", to.p_path); >> rval =3D 1; >> } >=20 > This changes from a minor misformatting to avoid a long line to even ug= lier > formatting with a split line. I agree 100%. I did it because of our hard 80-width cut-off. What would the proper style be? My inclination would be to wrap at the first comma but then it is even more odd. I find our 80-width cut-off to be strange when editors/tmux/window manager/etc can resize and wrap long lines alrea= dy. Actually I don't see a width restriction in style(9) at all but surely we have this rule documented somewhere. My guess is that it is inherited by KNF. > It is necessary to make such changes if you > use indent(1) to generate and check the changes -- otherwise, indent ke= eps Do you have an indent configuration I can use? > reporting the misformatting -- but since cp rarely went near indent it > may be better to keep its minor misformattings. >=20 >> Modified: head/bin/cp/utils.c >> =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=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D >> >> --- head/bin/cp/utils.c Mon Jun 8 19:13:04 2015 (r284162) >> +++ head/bin/cp/utils.c Mon Jun 8 19:24:18 2015 (r284163) >> ... >> -/* Small (default) buffer size in bytes. It's inefficient for this to= be >> - * smaller than MAXPHYS */ >> +/* >> + * Small (default) buffer size in bytes. It's inefficient for this to= be >> + * smaller than MAXPHYS. >> + */ >=20 > Still has unusual sentence break of 1 space. cp uses normal sentence > breaks I did a minimal effort on comments and didn't clean up grammar or breaks. I have not adopted 2 space breaks into my style(9) conformation y= et. >> @@ -345,7 +352,7 @@ setfile(struct stat *fs, int fd) >> fdval =3D fd !=3D -1; >> islink =3D !fdval && S_ISLNK(fs->st_mode); >> fs->st_mode &=3D S_ISUID | S_ISGID | S_ISVTX | >> - S_IRWXU | S_IRWXG | S_IRWXO; >> + S_IRWXU | S_IRWXG | S_IRWXO; >=20 > Here the formatting was reasonable, but it was in gnu style and was har= d to > maintain since it is not supported by indent(1). It is still hard to > maintain, > since it has fancy splitting earlier than necessary to put the S_IS* an= d > S_IR* parts of the expressions on separate lines. indent(1) cannot > reproduce > this splitting. Also, with the normal indentation of the condinuation > line, > the fancy splitting is not so readable. I'm do not see how this was proper before or how it is worse now. The indentation is tabs and then 4 spaces. I don't see exceptions to this in style(9) or in other code. >=20 >> @@ -543,8 +550,10 @@ usage(void) >> { >> >> (void)fprintf(stderr, "%s\n%s\n", >> -"usage: cp [-R [-H | -L | -P]] [-f | -i | -n] [-alpsvx] source_file >> target_file", >> -" cp [-R [-H | -L | -P]] [-f | -i | -n] [-alpsvx] source_file >> ... " >> -"target_directory"); >> + "usage: cp [-R [-H | -L | -P]] [-f | -i | -n] [-alpsvx] " >> + "source_file target_file", >> + " cp [-R [-H | -L | -P]] [-f | -i | -n] [-alpsvx] " >> + "source_file ... " >> + "target_directory"); >> exit(EX_USAGE); >> } >=20 > This breaks the careful outdentation and obfuscates the strings. The Again, this broke the 80-width limit. I preferred the old way but was going on down the 80-width line on my screen fixing violations. I suggest we update our styles to not require this awful wrapping. It makes `grep -r` very difficult when strings are split up. Perhaps I am mistaken on the rule but we have a lot of code that needlessly wraps earl= y. Regards, Bryan Drewery --5TnR9hbiIaAbEsS7FB5tPK1o5mXoQammU Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQEcBAEBAgAGBQJVi3RcAAoJEDXXcbtuRpfPpcEIAJuqfa0rdLGVUG0UlN5mlm22 Z5nUUisEI0sGA2Y3glks+d7vU8qktNVahzQPkqXh4ESb1MX/k1kKWEJyhCbCVmpB 3zazlkyjFEQxULWM24zaQvf+YCDtoDCFsnn3ka3j5ueLzFk72uVguKU5Ss6niUyU PCUbF4Z6RXed5K7HI0WUSbFsLtJWPYewZZc/7X65j1HAAKXAOlMCennXqLoFSFzE Y5LqVh7syrVbhgoc+7eD8J2+8M/QUsTtOBe3FBWaMS3mQSs+Sqe/xXwkcFBUSWTw V2jfHY9XKttQAJRivRkI+iUiUYMxPocoAireOxu29kXuaPEa8McjIIiyDgqgLeM= =yF7A -----END PGP SIGNATURE----- --5TnR9hbiIaAbEsS7FB5tPK1o5mXoQammU--