From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 11853 invoked from network); 29 Nov 2004 16:44:05 -0000 Received: from news.dotsrc.org (HELO a.mx.sunsite.dk) (130.225.247.88) by ns1.primenet.com.au with SMTP; 29 Nov 2004 16:44:05 -0000 Received: (qmail 94851 invoked from network); 29 Nov 2004 16:43:58 -0000 Received: from sunsite.dk (130.225.247.90) by a.mx.sunsite.dk with SMTP; 29 Nov 2004 16:43:58 -0000 Received: (qmail 13706 invoked by alias); 29 Nov 2004 16:43:53 -0000 Mailing-List: contact zsh-workers-help@sunsite.dk; run by ezmlm Precedence: bulk X-No-Archive: yes X-Seq: 20595 Received: (qmail 13687 invoked from network); 29 Nov 2004 16:43:52 -0000 Received: from unknown (HELO a.mx.sunsite.dk) (130.225.247.88) by sunsite.dk with SMTP; 29 Nov 2004 16:43:52 -0000 Received: (qmail 93731 invoked from network); 29 Nov 2004 16:42:55 -0000 Received: from dsl3-63-249-88-2.cruzio.com (HELO binome.blorf.net) (63.249.88.2) by a.mx.sunsite.dk with SMTP; 29 Nov 2004 16:42:54 -0000 Received: by binome.blorf.net (Postfix, from userid 1000) id A12DC2649; Mon, 29 Nov 2004 08:42:53 -0800 (PST) Date: Mon, 29 Nov 2004 08:42:53 -0800 From: Wayne Davison To: Peter Stephenson Cc: Zsh hackers list Subject: zsh coding standards Message-ID: <20041129164253.GA539@blorf.net> References: <20041129115700.GA4569@sc> <200411291210.iATCA9wt020885@news01.csr.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="tKW2IUtsqtDRztdT" Content-Disposition: inline In-Reply-To: <200411291210.iATCA9wt020885@news01.csr.com> User-Agent: Mutt/1.5.6+20040907i X-Spam-Checker-Version: SpamAssassin 2.63 on a.mx.sunsite.dk X-Spam-Level: X-Spam-Status: No, hits=-0.0 required=6.0 tests=BAYES_44 autolearn=no version=2.63 X-Spam-Hits: -0.0 --tKW2IUtsqtDRztdT Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Mon, Nov 29, 2004 at 12:10:09PM +0000, Peter Stephenson wrote: > Thanks, I've committed this, slightly abbreviated to fit in with zsh's > dense and opaque coding standards :-/. Speaking of coding standards, it has only been fairly recently (perhaps this year?) that you have started to put braces on the line following an "if", "while", etc. Is this a conscious decision to change zsh's coding style? I personally prefer seeing the braces on the same line as the controlling statement (except for the start of a function). Some other things I think improve readability: Use "if (chdir(dir) == 0)" to test for success instead of "if (!chdir(dir))" since the latter reads "NOT chdir", which reads like we're testing for failure. Similarly, use "if (chdir(dir) < 0)" to test for failure instead of "if (chdir(dir))" since the latter reads like we're testing for success. Use "{}" on an empty loop instead of ";" to make it more obvious that no looping statements follow. Debate? Attached is a patch of the zchdir() function that contains only the style changes mentioned above. They won't be committed unless agreement is reached that they would be a good thing. Note also that I just checked in a fix for zchdir()'s function comment (it said it returns 0 on normal error when it really returns -1). I made a simple optimization of the error-return code at the same time. ..wayne.. --tKW2IUtsqtDRztdT Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="zchdir.patch" --- Src/compat.c 29 Nov 2004 16:26:12 -0000 1.14 +++ Src/compat.c 29 Nov 2004 16:28:03 -0000 @@ -387,8 +387,7 @@ zchdir(char *dir) int currdir = -2; for (;;) { - if (!*dir || !chdir(dir)) - { + if (!*dir || chdir(dir) == 0) { #ifdef HAVE_FCHDIR if (currdir >= 0) close(currdir); @@ -398,7 +397,7 @@ zchdir(char *dir) if ((errno != ENAMETOOLONG && errno != ENOMEM) || strlen(dir) < PATH_MAX) break; - for (s = dir + PATH_MAX - 1; s > dir && *s != '/'; s--); + for (s = dir + PATH_MAX - 1; s > dir && *s != '/'; s--) {} if (s == dir) break; #ifdef HAVE_FCHDIR @@ -406,7 +405,7 @@ zchdir(char *dir) currdir = open(".", O_RDONLY|O_NOCTTY); #endif *s = '\0'; - if (chdir(dir)) { + if (chdir(dir) < 0) { *s = '/'; break; } @@ -414,7 +413,7 @@ zchdir(char *dir) currdir = -1; #endif *s = '/'; - while (*++s == '/'); + while (*++s == '/') {} dir = s; } #ifdef HAVE_FCHDIR --tKW2IUtsqtDRztdT--