svn commit: r190919 - in head/sys: amd64/amd64 amd64/include i386/i386 i386/include

Bruce Evans 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.
>>
>> 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


More information about the svn-src-head mailing list