Date: Mon, 12 Feb 2018 11:55:21 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Warner Losh <imp@freebsd.org> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r329127 - head/sys/sys Message-ID: <20180212111457.R873@besplex.bde.org> In-Reply-To: <201802111745.w1BHjcWA050692@repo.freebsd.org> References: <201802111745.w1BHjcWA050692@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, 11 Feb 2018, Warner Losh wrote: > Log: > Consistent macro indentation is the hobgoblin of small minds > > Line up the macro definitions and names here like the machine/stdarg.h > files that this replaced. This is easier to read and also makes it > easier to match up with other includes. Also two space indent va_end > to match the rest of the surrounding if block. Any chance of using KNF style? The 2-column indent for cpp things is easier to read than the KNF 0-column indent for cpp things, but is inconsistent with the KNF 8-column indent for C things, and is harder to write and maintain. > Modified: head/sys/sys/_stdarg.h > ============================================================================== > --- head/sys/sys/_stdarg.h Sun Feb 11 16:35:56 2018 (r329126) > +++ head/sys/sys/_stdarg.h Sun Feb 11 17:45:38 2018 (r329127) > @@ -43,13 +43,13 @@ > #endif > > #ifdef __GNUCLIKE_BUILTIN_STDARG > - #define va_start(ap, last) __builtin_va_start((ap), (last)) > - #define va_arg(ap, type) __builtin_va_arg((ap), type) > - #define __va_copy(dest, src) __builtin_va_copy((dest), (src)) > + #define va_start(ap, last) __builtin_va_start((ap), (last)) > + #define va_arg(ap, type) __builtin_va_arg((ap), type) > + #define __va_copy(dest, src) __builtin_va_copy((dest), (src)) In KNF, '#define' is not indented, and the tab after '#define' puts the name in column 8. Here, '#define is indented by 2 columns, and the space after #define used to put the name in column 2+7+1 = 10; now it puts the name in column 16 which is too far to the right. > #if __ISO_C_VISIBLE >= 1999 > - #define va_copy(dest, src) __va_copy(dest, src) > + #define va_copy(dest, src) __va_copy(dest, src) Deeper nesting causes further problems. Now the '#define's are indented by 2+2 columns. The names used to be indented to match, but they are now indented uniformly to 16 (except for unconverted ones), so their indentation no longer reflects the nesting. For 5 levels of nesting, the #define's would be in column 10 and the tab would indent the names to column 24. > #endif > -#define va_end(ap) __builtin_va_end(ap) > + #define va_end(ap) __builtin_va_end(ap) This like was actually in KNF style. > #endif > > #if defined(lint) && !defined(va_start) There are many unchanged misformattings before and after the ones modified in the patch. These are now more inconsistent than before: before: - space instead of tab after #define for idempotency ifdef - space instead of tab after #define for _VA_LIST_DECLARED after: - no tabs in the lint section (4 #define's). One of these is long and would blow out to 92 columns with tabs instead of spaces. After fixing the indentation of #define, it still takes 83 columns. Other style bugs in this file: - _VA_LIST_DECLARED is declared before va_list is actually declared. This is illogical, and its bad style is not used for older #define's of this type - space after ! in comment on #endif for idempotency ifdef - garbage newline before EOF. This file scores 67% on knfom(1) after this commit, down from 68% before. Most of the errors detected are for the 2-column indentation of almost everything. After fixing this, the score is 95%. IIRC, indent(1) never reformats cpp directives. knfom(1) uses indent(1) so it doesn't notice bugs in cpp expressions. The score is only 95% because of bugs in indent(1). indent(1) fixes the cpp indentation and the garbage before EOF, but adds the following bugs: - change the tab in tyhe typedef to a space - indent the comment on the #endif for the idempotency ifdef. Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20180212111457.R873>