svn commit: r190919 - in head/sys: amd64/amd64 amd64/include
brde at optusnet.com.au
Sat Apr 11 18:17:54 UTC 2009
On Sat, 11 Apr 2009, Ed Schouten wrote:
> * Steve Kargl <sgk at troutmask.apl.washington.edu> wrote:
>> I thought Christoph and bde were still hashing out the correctness
>> of this patch.
> 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
- 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__.
- 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.
- 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__.
More information about the svn-src-head