qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: John Snow <jsnow@redhat.com>, qemu-devel@nongnu.org
Cc: qemu-trivial@nongnu.org, peter.maydell@linaro.org,
	qemu-block@nongnu.org, Paolo Bonzini <pbonzini@redhat.com>
Subject: Re: [Qemu-devel] [PATCH] scsi: Address spurious clang warning
Date: Tue, 27 Nov 2018 13:02:29 -0600	[thread overview]
Message-ID: <f2257e95-b17c-55d4-1d5f-cc09db3d6a58@redhat.com> (raw)
In-Reply-To: <20181127184929.20065-1-jsnow@redhat.com>

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 <jsnow@redhat.com>
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

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?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

  reply	other threads:[~2018-11-27 19:03 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-27 18:49 [Qemu-devel] [PATCH] scsi: Address spurious clang warning John Snow
2018-11-27 19:02 ` Eric Blake [this message]
2018-11-27 20:55   ` Peter Maydell
2018-11-27 21:08   ` John Snow
2018-11-28  0:37 ` Peter Maydell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f2257e95-b17c-55d4-1d5f-cc09db3d6a58@redhat.com \
    --to=eblake@redhat.com \
    --cc=jsnow@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-trivial@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).