Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 4 Feb 2004 08:31:48 +0100
From:      Pawel Jakub Dawidek <pjd@FreeBSD.org>
To:        Bruce Evans <bde@zeta.org.au>
Cc:        freebsd-arch@freebsd.org
Subject:   Re: Non-standard ;; and SYSINIT().
Message-ID:  <20040204073148.GD14639@garage.freebsd.pl>
In-Reply-To: <20040204172838.L1097@gamplex.bde.org>
References:  <20040203214530.GB14639@garage.freebsd.pl> <200402031720.20211.john@baldwin.cx> <20040204172838.L1097@gamplex.bde.org>

next in thread | previous in thread | raw e-mail | index | archive | help

--MAH+hnPXVZWQ5cD/
Content-Type: text/plain; charset=iso-8859-2
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Wed, Feb 04, 2004 at 06:02:45PM +1100, Bruce Evans wrote:
+> > > It looks like SYSINIT() macro is defined with trailing ;.
+> > > Maybe there was some reason to do so, but I assume that this is a bu=
g.
+> > > 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).
+>=20
+> 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).
+>=20
+> > > Here is a patch that fix this issue at least for SYSINIT():
+> > >
+> > > 	http://garage.freebsd.pl/patches/SYSINIT.patch
+>=20
+> 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

Have you count:
SYSINIT(..., ...,
    ...);
?
Or SYSINITs in other macros? They don't start with line start sometimes.
Anyway, I am able to compile LINT with my patch and SYSINIT() without
semicolon is treated as a bug.

+> > > 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 t=
o do
+> > autoindent.
+>=20
+> There is a PR or two about this (mostly from the point of view of editor=
s,
+> starting with one for gtags IIRC).
+>=20
+> 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.

So maybe this is good time to start such little, but big cleanups.
Now we have SYSINIT() which doesn't require semicol and SYSUNINIT()
which needs it.

If you will decide to do this, feel free to use my patch.

IMHO this is worth it, if we want to support other compilers in the future.

--=20
Pawel Jakub Dawidek                       http://www.FreeBSD.org
pjd@FreeBSD.org                           http://garage.freebsd.pl
FreeBSD committer                         Am I Evil? Yes, I Am!

--MAH+hnPXVZWQ5cD/
Content-Type: application/pgp-signature
Content-Disposition: inline

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.0.7 (FreeBSD)

iD8DBQFAIJ/kForvXbEpPzQRAs+HAKCYpHCDTTA0QdSrQbSMyLNbg/eZYwCglMtj
F8N3Sxb8wxRRfvU8pOHVdR0=
=5hfK
-----END PGP SIGNATURE-----

--MAH+hnPXVZWQ5cD/--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20040204073148.GD14639>