Date: Sun, 12 Apr 2009 04:17:25 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Ed Schouten <ed@80386.nl> Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org, Steve Kargl <sgk@troutmask.apl.washington.edu> Subject: Re: svn commit: r190919 - in head/sys: amd64/amd64 amd64/include i386/i386 i386/include Message-ID: <20090412033326.O4999@besplex.bde.org> In-Reply-To: <20090411165114.GV32098@hoeg.nl> References: <200904111401.n3BE1108088009@svn.freebsd.org> <20090411163528.GC46526@troutmask.apl.washington.edu> <20090411165114.GV32098@hoeg.nl>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sat, 11 Apr 2009, Ed Schouten wrote: > * Steve Kargl <sgk@troutmask.apl.washington.edu> wrote: >> I thought Christoph and bde were still hashing out the correctness >> of this patch. >> >> http://lists.freebsd.org/pipermail/freebsd-amd64/2009-April/012064.html > > Yes, so I've only committed a subset of changes of which there were no > major (or in my opinion valid) objections: > > - The construct that we use now works with many versions of GCC. There > is absolutely no reason why we should still try to support GCC <2.95. But should we ifdef for it? The old code did. Eleswhere we have ugly mazes of ifdefs that don't actually work since they are not ugly enough to be complete. > - There was also the discussion about __inline vs inline and __volatile > vs __volatile. As Christoph and I noticed, there is also a lot of > inconsistency between the usage of the keywords in the current sources > we have. Actually, current sources are remarkably consistent about this, at least in i386/include/*.h (header files are likely to be more carefully maintained so this is not very surprising). In i386/include/*.h, there are just the 2 inconsistencies that were pointed out in the discussion: before this commit: __volatile, etc.: - 71 lines match 'volatile' but not __volatile. All uses of plain volatile are for Standard C declarations - 126 lines match __volatile. All uses of __volatile are in `__asm __volatile' statements, except 2 in comments in in_cksum.h. BTW, these comments and probably their code are wrong. Christoph apparently hasn't looked at this or he would point it out more strongly than me :-). Some related bad code for checksums has been fixed by writing it as a single asm. - 0 lines match the Gnuish __asm__ or __volatile__. __asm, etc.: - 8 lines match asm but not __asm. Only 2 of these are in code (style bugs). The others are in comments or unrelated. Oops, one very bogus one is related: acpoca_machdep.h #define's asm as __asm, which I think only makes the style bugs in the code not be fatal. (Our non-use of plain asm is enforced except for bogusness like this by compiling with fairly strict CFLAGS so that plain asm is a syntax error.) - 18 lines match __asm but not '__asm __volatile'. I think __volatile is used excessively (we get most of the style consistency by blindly copying it for safety). - 1 of these lines is for the bogus #define mentioned above. - 10 of these lines are for correctly renaming labels. - 3 of these lines are in endian.h but 1 or 2 of them were recently replaced by C code due to work of Cristoph. I think not using __volatile for these is correct. - 4 of these lines are in profile.h for bogus code that has already been pointed out by Christoph. These lines need __volatile more than most, except it is insufficient. They need to be written using a single asm and more. __inline, etc.: - 6 lines match `inline' but not __inline. 4 are in comments and 2 give the inconsistencies pointed out in the discussion. - 183 lines match __inline. All of these match 'static __inline'. (These matches and the ones for `__asm __volatile' are literal, with no style bugs in the whitspace between the keywords.) - 0 lines match the Gnuish __inline__. Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20090412033326.O4999>