[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-trivial] [Qemu-devel] [PATCH] scsi: Address spurious clang war
From: |
John Snow |
Subject: |
Re: [Qemu-trivial] [Qemu-devel] [PATCH] scsi: Address spurious clang warning |
Date: |
Tue, 27 Nov 2018 16:08:13 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.3.0 |
On 11/27/18 2:02 PM, Eric Blake wrote:
> On 11/27/18 12:49 PM, John Snow wrote:
>> Some versions of Clang prior to 6.0 (and some builds of clang after,
>> such as 6.0.1-2.fc28) fail to recognize { 0 } as a valid initializer
>> for a struct with subobjects when -Wmissing-braces is enabled.
>>
>> https://bugs.llvm.org/show_bug.cgi?id=21689 and
>> https://reviews.llvm.org/rL314499 suggests this should be fixed in 6.0,
>> but it might not be the case for older versions or downstream versions.
>>
>> For now, follow the precedent of ebf2a499 and replace the standard { 0 }
>> with the accepted { } to silence this warning and allow the build to
>> work under clang 6.0.1-2.fc28, and builds prior to 6.0.
>>
>> Signed-off-by: John Snow <address@hidden>
>>
>
> Reviewed-by: Eric Blake <address@hidden>
>
> I'm okay if this goes into -rc3 as a build-fix; I'm also okay if it
> slips to 4.0.
>
>> ---
>>
>> What I am actually less clear on is why this appears to be a problem
>> only now; since the introduction of { 0 } was in 2.11. It might be
>> a regression only in the fedora distribution of Clang 6.0.
>
> Or even a redefinition of struct dm_ioctl in some header where you are
> just now picking up a new struct layout that tickles the Clang issue in
> relation to the previous layout (since it is possible to have two
> structs that are ABI-compatible but where only one of the two has a
> nested substruct).
>
>> +++ b/scsi/qemu-pr-helper.c
>> @@ -236,7 +236,7 @@ static void dm_init(void)
>> perror("Cannot open " CONTROL_PATH);
>> exit(1);
>> }
>> - struct dm_ioctl dm = { 0 };
>> + struct dm_ioctl dm = { };
>
> Random thought: would it be worth having "qemu/compiler.h" define a macro:
>
> #if ...broken clang
> #define ZERO_INIT {}
> #else
> #define ZERO_INIT {0}
> #endif
>
> and then rewrite all our '= { 0? }' initializers into '= ZERO_INIT'? Or
> is that aesthetically too ugly?
>
Obscures perfectly legitimate C code without solving anything, IMO. As
much code as can reflect "naked" C89/C99/GNU99, the better.
--js