Re: [Qemu-devel] [PATCH V2 2/6] libqblock type and structure defines

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1881 bytes --]

On 09/12/2012 09:33 PM, Eric Blake wrote:
>>  OK ,then I think
>> #if __GNUC__ >= 4
>> ....
>> #else
>>   [warn name space pollution may happen]
>> #endif
>> would be better.
> 
> It may be shorter, but it is definitely not better, at least not in the
> current context of qemu.  Using the short form will fail a -Werror
> build, unless you also write a patch to qemu's configure to quit
> supplying -Wundef during builds.  But as touching configure has a bigger
> impact to the overall qemu project, you're going to need a lot more
> buy-in from other developers that -Wundef is not helping qemu gain any
> portability, and that it is safe to ditch it (or get enough
> counter-arguments from other developers why qemu insists on the
> anachronistic style enforced by -Wundef, at which point you must comply
> and use the longer form).

On second thought, this _particular_ usage will never fail a -Wundef
-Werror build, precisely because -Wundef is a gcc warning, which impies
the warning is only ever useful in the same scenarios that the __GNUC__
macro is always defined (that is, __GNUC__ is undefined only on a
non-gcc compiler, but what non-gcc compiler supports -Wundef -Werror?).

But why should this line be the one exemption to the rules?  Either qemu
insists on the -Wundef style of coding (and you should use the long form
to conform to that style, on the off-chance that someone ever wants to
port to a non-gcc compiler, even in this one place where gcc can't warn
you about the violation of that style), or we should change the qemu
style (at which point, the short form is nicer here, but it also implies
the potential for cleaning up lots of other places to also use short
forms and rely on preprocessor 0 computation).

-- 
Eric Blake   eblake@redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 617 bytes --]