Re: [PATCH] nbd: fix uninitialized variable warning
On 1/8/2020 6:24 AM, Eric Blake wrote:
> On 1/5/20 7:54 PM, pannengy...@huawei.com wrote:
>> From: Pan Nengyuan
>>
>> Fixes:
>> /mnt/sdb/qemu/nbd/server.c: In function 'nbd_handle_request':
>> /mnt/sdb/qemu/nbd/server.c:2313:9: error: 'ret' may be used uninitialized in
>> this function [-Werror=maybe-uninitialized]
>> int ret;
>>
>> Reported-by: Euler Robot
>
> False positive in the robot - I cannot see any path where ret is used
> uninitialized. Closest might be the handling of NBD_CMD_BLOCK_STATUS, which
> looks like:
>
> if (a || b) {
> if (a) {
> ret = ...;
> if (ret < 0) {
> return ret;
> }
> }
> if (b) {
> ret = ...;
> if (ret < 0) {
> return ret;
> }
> }
> return ret;
> }
>
> In fact, those 'if (ret < 0)' tests are pointless, since nothing else really
> happens before the final return ret.
>
> If I'm right about this being what trips up the robot, does changing 'if (b)'
> into 'else' solve the problem, rather than adding an initializer? And if so,
> can we clean up the pointless code while at it?
Yes, you are right, Changing 'if(b)' to 'else' solves the problem.
I will change it and clean up the pointless code in next version.
Thanks.
>
>> Signed-off-by: Pan Nengyuan
>> ---
>> nbd/server.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/nbd/server.c b/nbd/server.c
>> index 24ebc1a805..7eb3de0842 100644
>> --- a/nbd/server.c
>> +++ b/nbd/server.c
>> @@ -2310,7 +2310,7 @@ static coroutine_fn int nbd_handle_request(NBDClient
>> *client,
>> NBDRequest *request,
>> uint8_t *data, Error **errp)
>> {
>> - int ret;
>> + int ret = 0;
>> int flags;
>> NBDExport *exp = client->exp;
>> char *msg;
>>
>
Re: [PATCH] nbd: fix uninitialized variable warning
On 1/5/20 7:54 PM, pannengy...@huawei.com wrote:
From: Pan Nengyuan
Fixes:
/mnt/sdb/qemu/nbd/server.c: In function 'nbd_handle_request':
/mnt/sdb/qemu/nbd/server.c:2313:9: error: 'ret' may be used uninitialized in
this function [-Werror=maybe-uninitialized]
int ret;
Reported-by: Euler Robot
False positive in the robot - I cannot see any path where ret is used
uninitialized. Closest might be the handling of NBD_CMD_BLOCK_STATUS,
which looks like:
if (a || b) {
if (a) {
ret = ...;
if (ret < 0) {
return ret;
}
}
if (b) {
ret = ...;
if (ret < 0) {
return ret;
}
}
return ret;
}
In fact, those 'if (ret < 0)' tests are pointless, since nothing else
really happens before the final return ret.
If I'm right about this being what trips up the robot, does changing 'if
(b)' into 'else' solve the problem, rather than adding an initializer?
And if so, can we clean up the pointless code while at it?
Signed-off-by: Pan Nengyuan
---
nbd/server.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/nbd/server.c b/nbd/server.c
index 24ebc1a805..7eb3de0842 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -2310,7 +2310,7 @@ static coroutine_fn int nbd_handle_request(NBDClient
*client,
NBDRequest *request,
uint8_t *data, Error **errp)
{
-int ret;
+int ret = 0;
int flags;
NBDExport *exp = client->exp;
char *msg;
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
[PATCH] nbd: fix uninitialized variable warning
From: Pan Nengyuan
Fixes:
/mnt/sdb/qemu/nbd/server.c: In function 'nbd_handle_request':
/mnt/sdb/qemu/nbd/server.c:2313:9: error: 'ret' may be used uninitialized in
this function [-Werror=maybe-uninitialized]
int ret;
Reported-by: Euler Robot
Signed-off-by: Pan Nengyuan
---
nbd/server.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/nbd/server.c b/nbd/server.c
index 24ebc1a805..7eb3de0842 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -2310,7 +2310,7 @@ static coroutine_fn int nbd_handle_request(NBDClient
*client,
NBDRequest *request,
uint8_t *data, Error **errp)
{
-int ret;
+int ret = 0;
int flags;
NBDExport *exp = client->exp;
char *msg;
--
2.21.0.windows.1
Re: [Qemu-devel] [PATCH] nbd: fix uninitialized variable warning
Patchew URL:
https://patchew.org/QEMU/20190716084240.17594-1-marcandre.lur...@redhat.com/
Hi,
This series failed the asan build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.
=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-fedora V=1 NETWORK=1
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===
CC audio/noaudio.o
CC audio/wavaudio.o
CC audio/mixeng.o
/tmp/qemu-test/src/block/nbd.c:710:30: error: suggest braces around
initialization of subobject [-Werror,-Wmissing-braces]
NBDReply local_reply = { 0, };
^
{}
The full log is available at
http://patchew.org/logs/20190716084240.17594-1-marcandre.lur...@redhat.com/testing.asan/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com
Re: [Qemu-devel] [PATCH] nbd: fix uninitialized variable warning
Hi
On Tue, Jul 16, 2019 at 1:19 PM Philippe Mathieu-Daudé
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..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..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
>
> > Signed-off-by: Marc-André Lureau
> > ---
> > 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é
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
Re: [Qemu-devel] [PATCH] nbd: fix uninitialized variable warning
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..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..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].
Are you using that, or the latest GCC emit a warning even with no -O3?
Personally I'd add:
Fixes: 86f8cdf3db8
Reported-by: Thomas Huth
> Signed-off-by: Marc-André Lureau
> ---
> 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é
> 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
[Qemu-devel] [PATCH] nbd: fix uninitialized variable warning
../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..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..magic' was declared here
NBDReply local_reply;
^~~
cc1: all warnings being treated as errors
Signed-off-by: Marc-André Lureau
---
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, };
NBDStructuredReplyChunk *chunk;
Error *local_err = NULL;
if (s->quit) {
--
2.22.0.428.g6d5b264208
