[Qemu-devel] [PATCH for-2.10???] fix build failure in nbd_read_reply_entry()

[Qemu-devel] [PATCH for-2.10???] fix build failure in nbd_read_reply_entry()

[Igor Mammedov]

travis builds fail at HEAD at rc3 master with

  block/nbd-client.c: In function ‘nbd_read_reply_entry’:
  block/nbd-client.c:110:8: error: ‘ret’ may be used uninitialized in this function [-Werror=uninitialized]

fix it by initializing 'ret' to 0

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 block/nbd-client.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/nbd-client.c b/block/nbd-client.c
index 422ecb4..02c8e20 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -70,7 +70,7 @@ static coroutine_fn void nbd_read_reply_entry(void *opaque)
 {
     NBDClientSession *s = opaque;
     uint64_t i;
-    int ret;
+    int ret = 0;
     Error *local_err = NULL;
 
     while (!s->quit) {
-- 
2.7.4

Re: [Qemu-devel] [PATCH for-2.10???] fix build failure in nbd_read_reply_entry()

[Paolo Bonzini]

On 17/08/2017 18:14, Igor Mammedov wrote:
> travis builds fail at HEAD at rc3 master with
> 
>   block/nbd-client.c: In function ‘nbd_read_reply_entry’:
>   block/nbd-client.c:110:8: error: ‘ret’ may be used uninitialized in this function [-Werror=uninitialized]
> 
> fix it by initializing 'ret' to 0

This is a false positive, but it's understandably impossible for the
compiler to figure it out.

Even though we disable -Werror on release builds, it may be worth fixing
this in 2.10 if it doesn't delay the release.  Peter, what do you think
about applying this on top of -rc3 without doing a fourth candidate?

Paolo

> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  block/nbd-client.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/block/nbd-client.c b/block/nbd-client.c
> index 422ecb4..02c8e20 100644
> --- a/block/nbd-client.c
> +++ b/block/nbd-client.c
> @@ -70,7 +70,7 @@ static coroutine_fn void nbd_read_reply_entry(void *opaque)
>  {
>      NBDClientSession *s = opaque;
>      uint64_t i;
> -    int ret;
> +    int ret = 0;
>      Error *local_err = NULL;
>  
>      while (!s->quit) {
> 

Re: [Qemu-devel] [PATCH for-2.10???] fix build failure in nbd_read_reply_entry()

[Peter Maydell]

On 17 August 2017 at 17:16, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 17/08/2017 18:14, Igor Mammedov wrote:
>> travis builds fail at HEAD at rc3 master with
>>
>>   block/nbd-client.c: In function ‘nbd_read_reply_entry’:
>>   block/nbd-client.c:110:8: error: ‘ret’ may be used uninitialized in this function [-Werror=uninitialized]
>>
>> fix it by initializing 'ret' to 0
>
> This is a false positive, but it's understandably impossible for the
> compiler to figure it out.
>
> Even though we disable -Werror on release builds, it may be worth fixing
> this in 2.10 if it doesn't delay the release.  Peter, what do you think
> about applying this on top of -rc3 without doing a fourth candidate?

I don't like doing releases which haven't had an rc,
but we've had abbreviated "just a couple of days" rc-to-final
cycles before.

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-2.10???] fix build failure in nbd_read_reply_entry()

[Paolo Bonzini]

On 17/08/2017 18:17, Peter Maydell wrote:
> On 17 August 2017 at 17:16, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> On 17/08/2017 18:14, Igor Mammedov wrote:
>>> travis builds fail at HEAD at rc3 master with
>>>
>>>   block/nbd-client.c: In function ‘nbd_read_reply_entry’:
>>>   block/nbd-client.c:110:8: error: ‘ret’ may be used uninitialized in this function [-Werror=uninitialized]
>>>
>>> fix it by initializing 'ret' to 0
>>
>> This is a false positive, but it's understandably impossible for the
>> compiler to figure it out.
>>
>> Even though we disable -Werror on release builds, it may be worth fixing
>> this in 2.10 if it doesn't delay the release.  Peter, what do you think
>> about applying this on top of -rc3 without doing a fourth candidate?
> 
> I don't like doing releases which haven't had an rc,
> but we've had abbreviated "just a couple of days" rc-to-final
> cycles before.

It's just a matter of "looking unpolished".  It doesn't deserve -rc4,
not even for just a couple of days.

Paolo

Re: [Qemu-devel] [PATCH for-2.10???] fix build failure in nbd_read_reply_entry()

[Peter Maydell]

On 17 August 2017 at 17:31, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 17/08/2017 18:17, Peter Maydell wrote:
>> On 17 August 2017 at 17:16, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>> On 17/08/2017 18:14, Igor Mammedov wrote:
>>>> travis builds fail at HEAD at rc3 master with
>>>>
>>>>   block/nbd-client.c: In function ‘nbd_read_reply_entry’:
>>>>   block/nbd-client.c:110:8: error: ‘ret’ may be used uninitialized in this function [-Werror=uninitialized]
>>>>
>>>> fix it by initializing 'ret' to 0
>>>
>>> This is a false positive, but it's understandably impossible for the
>>> compiler to figure it out.
>>>
>>> Even though we disable -Werror on release builds, it may be worth fixing
>>> this in 2.10 if it doesn't delay the release.  Peter, what do you think
>>> about applying this on top of -rc3 without doing a fourth candidate?
>>
>> I don't like doing releases which haven't had an rc,
>> but we've had abbreviated "just a couple of days" rc-to-final
>> cycles before.
>
> It's just a matter of "looking unpolished".  It doesn't deserve -rc4,
> not even for just a couple of days.

The purpose of having an rc4 is to avoid the chance of
messing up the change (which is possible, even if it's a pretty
remote chance). Having an rc4 gives us a window to catch and
fix that kind of error -- once we've tagged something as the
final release we don't get to do it over.

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-2.10???] fix build failure in nbd_read_reply_entry()

[Paolo Bonzini]

On 17/08/2017 18:37, Peter Maydell wrote:
> On 17 August 2017 at 17:31, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> On 17/08/2017 18:17, Peter Maydell wrote:
>>> On 17 August 2017 at 17:16, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>>> On 17/08/2017 18:14, Igor Mammedov wrote:
>>>>> travis builds fail at HEAD at rc3 master with
>>>>>
>>>>>   block/nbd-client.c: In function ‘nbd_read_reply_entry’:
>>>>>   block/nbd-client.c:110:8: error: ‘ret’ may be used uninitialized in this function [-Werror=uninitialized]
>>>>>
>>>>> fix it by initializing 'ret' to 0
>>>>
>>>> This is a false positive, but it's understandably impossible for the
>>>> compiler to figure it out.
>>>>
>>>> Even though we disable -Werror on release builds, it may be worth fixing
>>>> this in 2.10 if it doesn't delay the release.  Peter, what do you think
>>>> about applying this on top of -rc3 without doing a fourth candidate?
>>>
>>> I don't like doing releases which haven't had an rc,
>>> but we've had abbreviated "just a couple of days" rc-to-final
>>> cycles before.
>>
>> It's just a matter of "looking unpolished".  It doesn't deserve -rc4,
>> not even for just a couple of days.
> 
> The purpose of having an rc4 is to avoid the chance of
> messing up the change (which is possible, even if it's a pretty
> remote chance). Having an rc4 gives us a window to catch and
> fix that kind of error -- once we've tagged something as the
> final release we don't get to do it over.

Understood, I meant we can release with the issue unfixed.

Paolo

Re: [Qemu-devel] [PATCH for-2.10???] fix build failure in nbd_read_reply_entry()

[Eric Blake]

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

On 08/17/2017 11:14 AM, Igor Mammedov wrote:
> travis builds fail at HEAD at rc3 master with
> 
>   block/nbd-client.c: In function ‘nbd_read_reply_entry’:
>   block/nbd-client.c:110:8: error: ‘ret’ may be used uninitialized in this function [-Werror=uninitialized]
> 
> fix it by initializing 'ret' to 0
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  block/nbd-client.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

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

I concur that it is a false positive, introduced by commit 72b6ffc7; and
that it should not affect tarball builds where we do not default to
turning on -Werror (other than an annoying warning). This feels to me to
be in the same category as a couple other patches we have already
identified, where it doesn't deserve spinning up -rc4 by itself, but if
we do have a major bug to fix, then including this one at the same time
is fine.

I'll defer to Peter's judgment on whether to apply it for 2.10;
meanwhile, I'll stick it on my NBD tree for 2.11 if it does not get
applied for 2.10.

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


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

Re: [Qemu-devel] [PATCH for-2.10???] fix build failure in nbd_read_reply_entry()

[Peter Maydell]

On 17 August 2017 at 17:14, Igor Mammedov <imammedo@redhat.com> wrote:
> travis builds fail at HEAD at rc3 master with
>
>   block/nbd-client.c: In function ‘nbd_read_reply_entry’:
>   block/nbd-client.c:110:8: error: ‘ret’ may be used uninitialized in this function [-Werror=uninitialized]
>
> fix it by initializing 'ret' to 0
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> --

Applied to master for 2.10, since we needed to run an rc4 anyway.

thanks
-- PMM