Date: Wed, 4 Feb 2004 18:02:45 +1100 (EST) From: Bruce Evans <bde@zeta.org.au> To: John Baldwin <john@baldwin.cx> Cc: freebsd-arch@freebsd.org Subject: Re: Non-standard ;; and SYSINIT(). Message-ID: <20040204172838.L1097@gamplex.bde.org> In-Reply-To: <200402031720.20211.john@baldwin.cx> References: <20040203214530.GB14639@garage.freebsd.pl> <200402031720.20211.john@baldwin.cx>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, 3 Feb 2004, John Baldwin wrote: > On Tuesday 03 February 2004 04:45 pm, Pawel Jakub Dawidek wrote: > > Hello. > > > > It looks like SYSINIT() macro is defined with trailing ;. > > Maybe there was some reason to do so, but I assume that this is a bug. > > There are many, many calls where an extra ; is added after SYSINIT(). > > SYSUNINIT() is defined without trailing ; ... > > > > This will be ok, but ;; is not supported by ICO C (gcc -pedantic > > tell me that). This is a bug in the calls with the extra semicolon. All calls in the original SYSINIT() mistake^W commit didn't have it, and I fixed all the extra semicolons that were there many years ago. Unfortunately, the kernel is not usually compiled with C compilers so bugs like this keep coming back. An extra "," at the end of enum lists and types smaller than int in bitfields are other favorites (the former is pedantic and is only required for C90, but the latter is serious since it gives unportabilities at both compile and runtime). > > Here is a patch that fix this issue at least for SYSINIT(): > > > > http://garage.freebsd.pl/patches/SYSINIT.patch This seems to be missing a few. A quick grep shows 111 lines matching "^-SYSINIT" in it, but there are 244 lines matching "^SYSINIT" in .c files in the kernel; among the latter there are 144 currently correct lines without the semicolon and 71 lines with it. I prefer not to make cosmetic changes in large amounts of non-broken code when it outweighs the broken code and was sort of waiting for the broken code to outweigh the non-broken code (or better, for SYSINIT to mostly go away) before changing this. Unfortunately, SYSINIT is not going away; RELENG_3: 112 lines without semicolon 20 with it RELENG_4: 80 lines without semicolon 46 with it -current: 144 lines without semicolon 71 with it > > The most important part is a change in sys/kernel.h, that removes > > trailing ; from SYSINIT() definition: > > > > - DATA_SET(sysinit_set,uniquifier ## _sys_init); > > + DATA_SET(sysinit_set,uniquifier ## _sys_init) > > > > AND REMEMBER! I'm not going to commit it (without strong approvals)!:) > > Yes, please. SYSINIT() without ;'s confuse "smart" editors that try to do > autoindent. There is a PR or two about this (mostly from the point of view of editors, starting with one for gtags IIRC). The queue FOREACH macros are similar mistakes at the level of editors. You have to add {} to make short loops even uglier to get indent(1) to accidentally understand them. Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20040204172838.L1097>