> Module Name: src > Committed By: lukem > Date: Sat Aug 1 02:45:36 UTC 2020 > > Modified Files: > src/share/misc: style > > Log Message: > style: prefer braces for single statement control statements > > Prefer to use { braces } around single statements after > control statements, instead of discouraging them. > > Per discussion on tech-userlevel & tech-kern, where the significant > majority of developers who responded (including current and former > core members) prefer this new style.
Hmm...that's not the conclusion I got from the thread. What you proposed (https://mail-index.netbsd.org/tech-userlevel/2020/07/12/msg012536.html), and got consensus on, was: - discourage braces around single statements + permit braces around single statements What you committed was: - discourage braces around single statements + prefer braces around single statements and add braces to all examples At least two core members (me and kre) preferred the change you originally proposed over the change you committed. Personally I feel that braces around short statements hurt legibility by adding unnecessary visual clutter, and make it more cumbersome to have consistent patterns like if (foo() == -1) goto fail0; if ((x = bar()) == -1) goto fail1; if (baz() == -1) goto fail2; which makes it more tempting to get clever with shortcuts for error branches or with reversing the sense of the branch, and we have too many bugs with clever shortcuts in error branches already. We don't have a `goto fail' problem in NetBSD -- if we did, our toolchain would detect it with -Werror=misleading-indentation, as I just confirmed experimentally. (Same goes for macros that expand to multiple statements, with -Werror=multistatement-macros.) Can you please restore this to the change you originally suggested, along the lines of the attached patch?
Index: share/misc/style =================================================================== RCS file: /cvsroot/src/share/misc/style,v retrieving revision 1.56 diff -p -p -u -r1.56 style --- share/misc/style 1 Aug 2020 02:45:35 -0000 1.56 +++ share/misc/style 1 Aug 2020 22:54:53 -0000 @@ -241,9 +241,8 @@ main(int argc, char *argv[]) errno = 0; num = strtol(optarg, &ep, 10); if (num <= 0 || *ep != '\0' || (errno == ERANGE && - (num == LONG_MAX || num == LONG_MIN)) ) { + (num == LONG_MAX || num == LONG_MIN)) ) errx(1, "illegal number -- %s", optarg); - } break; case '?': default: @@ -256,16 +255,16 @@ main(int argc, char *argv[]) /* * Space after keywords (while, for, return, switch). - * Braces are preferred for control statements - * with only a single statement. + * + * Braces around single-line bodies are optional; use discretion. * * Forever loops are done with for's, not while's. */ - for (p = buf; *p != '\0'; ++p) { + for (p = buf; *p != '\0'; ++p) continue; /* Explicit no-op */ - } for (;;) { - stmt; + stmt1; + stmt2; } /* @@ -317,9 +316,8 @@ main(int argc, char *argv[]) } /* No spaces after function names. */ - if ((result = function(a1, a2, a3, a4)) == NULL) { + if ((result = function(a1, a2, a3, a4)) == NULL) exit(1); - } /* * Unary operators don't require spaces, binary operators do. @@ -397,12 +395,10 @@ function(int a1, int a2, float fl, int a * * Use err/warn(3), don't roll your own! */ - if ((four = malloc(sizeof(*four))) == NULL) { + if ((four = malloc(sizeof(*four))) == NULL) err(1, NULL); - } - if ((six = (int *)overflow()) == NULL) { + if ((six = (int *)overflow()) == NULL) errx(1, "Number overflowed."); - } /* No parentheses are needed around the return value. */ return eight; @@ -426,9 +422,8 @@ dirinfo(const char *p, struct stat *sb, _DIAGASSERT(p != NULL); _DIAGASSERT(filedesc != -1); - if (stat(p, sb) < 0) { + if (stat(p, sb) < 0) err(1, "Unable to stat %s", p); - } /* * To printf quantities that might be larger than "long", include