svn commit: r216143 - in head: share/man/man9 sys/amd64/include sys/arm/include sys/i386/include sys/ia64/include sys/mips/include sys/pc98/include sys/powerpc/include sys/sparc64/include sys/sun4v...

Bruce Evans brde at optusnet.com.au
Sat Dec 4 10:06:37 UTC 2010


On Fri, 3 Dec 2010, Eygene Ryabinkin wrote:

> Sat, Dec 04, 2010 at 05:01:55AM +1100, Bruce Evans wrote:
>> On Fri, 3 Dec 2010, Bruce Cran wrote:
>>> Log:
>>>  Revert r216134. This checkin broke platforms where bus_space are macros:
>>>  they need to be a single statement, and do { } while (0) doesn't
>>>  work in this situation so revert until a solution can be devised.
>>
>> Surprising that do-while doesn't work.
>
> Prior to the revert, something like "a = bus_space_read_multi_1(...)"
> will generate improper code like "a = KASSERT(); __bs_nonsigle(XXX);"
> and making "do { KASSERT(); __bs_nonsingle(XXX); } while(0)" won't
> help either, since we can't generally assign the compound statement
> to the lvalue.

Ah, the functions actually return something :-).

>> I just noticed the following possibly more serious problems for the macro
>> versions:
>>
>> - the `c' arg is missing parentheses in the KASSERT()
>> - the `c' arg is now evaluated twice.  This turns safe macros into unsafe
>>    ones.
>
> Perhaps we can define the macros as
> {{{
> #define	bus_space_read_multi_1(t, h, o, a, c)				({\
> 	size_t count = (c);						\
> 	KASSERT(count != 0, ("bus_space_read_multi_1: count == 0"));	\
> 	__bs_nonsingle(rm,1,(t),(h),(o),(a),count);			\
> 	})
> }}}
>
> This will both allow to avoid unsafety and will make this statement
> to be the correct assignment for any compiler that supports the
> "braced-groups within expressions" GNU extension.  GNU C, Clang
> and Intel C both support it (but not with -pedantic -ansi -Werror
> flag combo).

This is why amd64 and i386 use __extension when they use
statement-expressions.  grep in -current shows 32 .h files under /sys
matching "({", and only 7 of these files use __extension.  It is mostly
headers visible in userland that are careful.  sparc64 and sun4v
atomic.h seem to be the only headers that both use ({ and are used in
userland.  For .c files, statement-expressions are remarkably little-used
-- there are more line 32 lines total matching "({", and 0 lines
matching "__extension".

> But, probably, the inline function will be better here from the
> portability point of view, since it is supported by the C standard
> and braced-groups -- aren't.
>
> So, the question is "why these statements were made to be
> macros at some platforms?".

I guess it is just because they seemed to be simple enough to be macros.
Macros and inline have different technical advantages but I don't see
any important ones here.

Bruce


More information about the svn-src-all mailing list