Zsh Mailing List Archive
Messages sorted by: Reverse Date, Date, Thread, Author

zsh coding standards



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..
--- 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


Messages sorted by: Reverse Date, Date, Thread, Author