From owner-cvs-all Tue Aug 31 11:21:25 1999 Delivered-To: cvs-all@freebsd.org Received: from peak.mountin.net (peak.mountin.net [207.227.119.2]) by hub.freebsd.org (Postfix) with ESMTP id E0DD8159A6; Tue, 31 Aug 1999 11:21:15 -0700 (PDT) (envelope-from jeff-ml@mountin.net) Received: (from daemon@localhost) by peak.mountin.net (8.9.1/8.9.1) id NAA09913; Tue, 31 Aug 1999 13:20:49 -0500 (CDT) (envelope-from jeff-ml@mountin.net) Received: from dial-60.tnt1.rac.cyberlynk.net(209.224.182.60) by peak.mountin.net via smap (V1.3) id sma009911; Tue Aug 31 13:20:37 1999 Message-Id: <3.0.3.32.19990831132028.0155c100@207.227.119.2> X-Sender: jeff-ml@207.227.119.2 X-Mailer: QUALCOMM Windows Eudora Pro Version 3.0.3 (32) Date: Tue, 31 Aug 1999 13:20:28 -0500 To: John Hay , obrien@FreeBSD.ORG From: "Jeffrey J. Mountin" Subject: Re: cvs commit: src/bin/rm rm.1 rm.c Cc: cvs-committers@FreeBSD.ORG, cvs-all@FreeBSD.ORG In-Reply-To: <199908291927.VAA37960@zibbi.mikom.csir.co.za> References: <19990829114222.B14701@dragon.nuxi.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Sender: owner-cvs-all@FreeBSD.ORG Precedence: bulk At 09:27 PM 8/29/99 +0200, John Hay wrote: >> > > Reviewed by: obrien >> > >> > Either of you care to make any comments about this: (There is a second >> > report of it as well) >> >> Na. I'd rather just fix it. >> > >This seems to fix it for me. > >John >-- >John Hay -- John.Hay@mikom.csir.co.za > >Index: rm.c >=================================================================== >RCS file: /home/ncvs/src/bin/rm/rm.c,v >retrieving revision 1.22 >diff -u -r1.22 rm.c >--- rm.c 1999/08/29 02:20:26 1.22 >+++ rm.c 1999/08/29 18:57:59 >@@ -235,7 +235,7 @@ > switch (p->fts_info) { > case FTS_DP: > case FTS_DNR: >- if ((e=rmdir(p->fts_accpath)) || (fflag && errno == ENOENT)) { >+ if (!(e=rmdir(p->fts_accpath)) || (fflag && errno == ENOENT)) { > if (e == 0 && vflag) > (void)printf("%s\n", p->fts_accpath); > continue; Certainly don't care to start any style(9) wars here, but using '!' rather than '== [NULL|'\0'] isn't as obvious when reviewing code. Quite easy to miss. OTOH, using '!' is less typing and can be made more clear if a space were using in front eg: if ( !(e-rmdir(p->ftp_accpath)) This applies especially when there are several parenthesis in a complex comparison. I realize that using an explicit comparison adds a number of characters to the code (recalling past discussions on bloating the size of the source) and the increase in line lengths (to split or not to split). This makes sense of there is a space after the 'if' and elsewhere to increase clarity. I'm a neophyte to C, but surely anyone can miss the '!' when doing a quick review of code they are not initimate with. Certainly seen enough code to shudder at some practices, alibeit they are mostly located in the ports collection. Regardless it's not for me to judge, only suggest. Now, is the 'e=' supposed to be 'e=='? Must be -current code, so there might be a reason. Jeff Mountin - jeff@mountin.net Systems/Network Administrator FreeBSD - the power to serve '86 Yamaha MaxiumX (not FBSD powered) To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe cvs-all" in the body of the message