Re: [Qemu-devel] [PATCH] nbd: fix uninitialized variable warning

[Marc-André Lureau]

Hi

On Tue, Jul 16, 2019 at 1:19 PM Philippe Mathieu-Daudé
<address@hidden> wrote:
>
> Hi Marc-André,
>
> On 7/16/19 10:42 AM, Marc-André Lureau wrote:
> > ../block/nbd.c: In function 'nbd_co_request':
> > ../block/nbd.c:745:8: error: 'local_reply.type' may be used uninitialized 
> > in this function [-Werror=maybe-uninitialized]
> >      if (chunk->type == NBD_REPLY_TYPE_NONE) {
> >         ^
> > ../block/nbd.c:710:14: note: 'local_reply.type' was declared here
> >      NBDReply local_reply;
> >               ^~~~~~~~~~~
> > ../block/nbd.c:710:14: error: 'local_reply.flags' may be used uninitialized 
> > in this function [-Werror=maybe-uninitialized]
> > ../block/nbd.c:738:8: error: 'local_reply.<U4be0>.magic' may be used 
> > uninitialized in this function [-Werror=maybe-uninitialized]
> >      if (nbd_reply_is_simple(reply) || s->quit) {
> >         ^
> > ../block/nbd.c:710:14: note: 'local_reply.<U4be0>.magic' was declared here
> >      NBDReply local_reply;
> >               ^~~~~~~~~~~
> > cc1: all warnings being treated as errors
>
> Thomas reported this error when compiling with -O3 few months ago [1].

Thanks, I couldn't find a previous report.

> Are you using that, or the latest GCC emit a warning even with no -O3?

Right, the warning seems to appear with -O3 (I happen to have it with
mingw64-gcc-8.3.0-2.fc30.x86_64)

>
> Personally I'd add:
>
> Fixes: 86f8cdf3db8

Actually, it was there before, so I'll skip that.

> Reported-by: Thomas Huth <address@hidden>
>
> > Signed-off-by: Marc-André Lureau <address@hidden>
> > ---
> >  block/nbd.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/block/nbd.c b/block/nbd.c
> > index 81edabbf35..02eef09728 100644
> > --- a/block/nbd.c
> > +++ b/block/nbd.c
> > @@ -707,7 +707,7 @@ static bool nbd_reply_chunk_iter_receive(BDRVNBDState 
> > *s,
> >                                           void **payload)
> >  {
> >      int ret, request_ret;
> > -    NBDReply local_reply;
> > +    NBDReply local_reply = { 0, };
>
> Yesterday [2] Peter said: "= {}" is our standard struct-zero-initializer
> so we should prefer that.
>
> With {}:
> Reviewed-by: Philippe Mathieu-Daudé <address@hidden>

ok, thanks

>
> >      NBDStructuredReplyChunk *chunk;
> >      Error *local_err = NULL;
> >      if (s->quit) {
> >
>
> [1] https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg07007.html
> [2] https://lists.gnu.org/archive/html/qemu-devel/2019-07/msg03431.html