migration/qemu-file: Fix maybe uninitialized on qemu_get_buffer_in_place()

[PATCH] migration/qemu-file: Fix maybe uninitialized on qemu_get_buffer_in_place()

Posted by Wainer dos Santos Moschetta 3 years, 3 months ago

Fixed error when compiling migration/qemu-file.c with -Werror=maybe-uninitialized
as shown here:

../migration/qemu-file.c: In function 'qemu_get_buffer_in_place':
../migration/qemu-file.c:604:18: error: 'src' may be used uninitialized in this function [-Werror=maybe-uninitialized]
  604 |             *buf = src;
      |             ~~~~~^~~~~
cc1: all warnings being treated as errors

Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
---
Passed on CI: https://gitlab.com/wainersm/qemu/-/pipelines/247801576

 migration/qemu-file.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index be21518c57..d6e03dbc0e 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -595,7 +595,7 @@ size_t qemu_get_buffer_in_place(QEMUFile *f, uint8_t **buf, size_t size)
 {
     if (size < IO_BUF_SIZE) {
         size_t res;
-        uint8_t *src;
+        uint8_t *src = NULL;
 
         res = qemu_peek_buffer(f, &src, size, 0);
 
-- 
2.28.0

Re: [PATCH] migration/qemu-file: Fix maybe uninitialized on qemu_get_buffer_in_place()

Posted by Dr. David Alan Gilbert 3 years, 3 months ago

* Wainer dos Santos Moschetta (wainersm@redhat.com) wrote:
> Fixed error when compiling migration/qemu-file.c with -Werror=maybe-uninitialized
> as shown here:
> 
> ../migration/qemu-file.c: In function 'qemu_get_buffer_in_place':
> ../migration/qemu-file.c:604:18: error: 'src' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>   604 |             *buf = src;
>       |             ~~~~~^~~~~
> cc1: all warnings being treated as errors
> 
> Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>

Yes, I think I had a discussion with someone about this recently but
can't find it; the compiler is technically correct, but the only time
it's unitialised is the case where it's result doesn't matter.

Still, to shut the compiler up:


Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> ---
> Passed on CI: https://gitlab.com/wainersm/qemu/-/pipelines/247801576
> 
>  migration/qemu-file.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/migration/qemu-file.c b/migration/qemu-file.c
> index be21518c57..d6e03dbc0e 100644
> --- a/migration/qemu-file.c
> +++ b/migration/qemu-file.c
> @@ -595,7 +595,7 @@ size_t qemu_get_buffer_in_place(QEMUFile *f, uint8_t **buf, size_t size)
>  {
>      if (size < IO_BUF_SIZE) {
>          size_t res;
> -        uint8_t *src;
> +        uint8_t *src = NULL;
>  
>          res = qemu_peek_buffer(f, &src, size, 0);
>  
> -- 
> 2.28.0
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH] migration/qemu-file: Fix maybe uninitialized on qemu_get_buffer_in_place()

Posted by Philippe Mathieu-Daudé 3 years, 3 months ago

On 1/28/21 6:16 PM, Dr. David Alan Gilbert wrote:
> * Wainer dos Santos Moschetta (wainersm@redhat.com) wrote:
>> Fixed error when compiling migration/qemu-file.c with -Werror=maybe-uninitialized
>> as shown here:
>>
>> ../migration/qemu-file.c: In function 'qemu_get_buffer_in_place':
>> ../migration/qemu-file.c:604:18: error: 'src' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>>   604 |             *buf = src;
>>       |             ~~~~~^~~~~
>> cc1: all warnings being treated as errors
>>
>> Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
> 
> Yes, I think I had a discussion with someone about this recently but
> can't find it;

Maybe with Thomas, he reported that 2 years ago when building with -O3:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg599522.html

> the compiler is technically correct, but the only time
> it's unitialised is the case where it's result doesn't matter.
> 
> Still, to shut the compiler up:
> 
> 
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> 
>> ---
>> Passed on CI: https://gitlab.com/wainersm/qemu/-/pipelines/247801576
>>
>>  migration/qemu-file.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/migration/qemu-file.c b/migration/qemu-file.c
>> index be21518c57..d6e03dbc0e 100644
>> --- a/migration/qemu-file.c
>> +++ b/migration/qemu-file.c
>> @@ -595,7 +595,7 @@ size_t qemu_get_buffer_in_place(QEMUFile *f, uint8_t **buf, size_t size)
>>  {
>>      if (size < IO_BUF_SIZE) {
>>          size_t res;
>> -        uint8_t *src;
>> +        uint8_t *src = NULL;
>>  
>>          res = qemu_peek_buffer(f, &src, size, 0);
>>  
>> -- 
>> 2.28.0
>>

Re: [PATCH] migration/qemu-file: Fix maybe uninitialized on qemu_get_buffer_in_place()

Posted by Wainer dos Santos Moschetta 3 years, 3 months ago

Hi,

On 1/28/21 3:07 PM, Philippe Mathieu-Daudé wrote:
> On 1/28/21 6:16 PM, Dr. David Alan Gilbert wrote:
>> * Wainer dos Santos Moschetta (wainersm@redhat.com) wrote:
>>> Fixed error when compiling migration/qemu-file.c with -Werror=maybe-uninitialized
>>> as shown here:
>>>
>>> ../migration/qemu-file.c: In function 'qemu_get_buffer_in_place':
>>> ../migration/qemu-file.c:604:18: error: 'src' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>>>    604 |             *buf = src;
>>>        |             ~~~~~^~~~~
>>> cc1: all warnings being treated as errors
>>>
>>> Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
>> Yes, I think I had a discussion with someone about this recently but
>> can't find it;
> Maybe with Thomas, he reported that 2 years ago when building with -O3:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg599522.html


I saw that error when compiling QEMU with -O3 as well.


>
>> the compiler is technically correct, but the only time
>> it's unitialised is the case where it's result doesn't matter.
>>
>> Still, to shut the compiler up:
>>
>>
>> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>


Thanks David!


>>
>>> ---
>>> Passed on CI: https://gitlab.com/wainersm/qemu/-/pipelines/247801576
>>>
>>>   migration/qemu-file.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/migration/qemu-file.c b/migration/qemu-file.c
>>> index be21518c57..d6e03dbc0e 100644
>>> --- a/migration/qemu-file.c
>>> +++ b/migration/qemu-file.c
>>> @@ -595,7 +595,7 @@ size_t qemu_get_buffer_in_place(QEMUFile *f, uint8_t **buf, size_t size)
>>>   {
>>>       if (size < IO_BUF_SIZE) {
>>>           size_t res;
>>> -        uint8_t *src;
>>> +        uint8_t *src = NULL;
>>>   
>>>           res = qemu_peek_buffer(f, &src, size, 0);
>>>   
>>> -- 
>>> 2.28.0
>>>

Re: [PATCH] migration/qemu-file: Fix maybe uninitialized on qemu_get_buffer_in_place()

Posted by Dr. David Alan Gilbert 3 years, 3 months ago

* Philippe Mathieu-Daudé (philmd@redhat.com) wrote:
> On 1/28/21 6:16 PM, Dr. David Alan Gilbert wrote:
> > * Wainer dos Santos Moschetta (wainersm@redhat.com) wrote:
> >> Fixed error when compiling migration/qemu-file.c with -Werror=maybe-uninitialized
> >> as shown here:
> >>
> >> ../migration/qemu-file.c: In function 'qemu_get_buffer_in_place':
> >> ../migration/qemu-file.c:604:18: error: 'src' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> >>   604 |             *buf = src;
> >>       |             ~~~~~^~~~~
> >> cc1: all warnings being treated as errors
> >>
> >> Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
> > 
> > Yes, I think I had a discussion with someone about this recently but
> > can't find it;
> 
> Maybe with Thomas, he reported that 2 years ago when building with -O3:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg599522.html

I've got this vague memory I've had a conversation much much more
recently; like in the last month or two but I can't find it.

Dave

> > the compiler is technically correct, but the only time
> > it's unitialised is the case where it's result doesn't matter.
> > 
> > Still, to shut the compiler up:
> > 
> > 
> > Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > 
> >> ---
> >> Passed on CI: https://gitlab.com/wainersm/qemu/-/pipelines/247801576
> >>
> >>  migration/qemu-file.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/migration/qemu-file.c b/migration/qemu-file.c
> >> index be21518c57..d6e03dbc0e 100644
> >> --- a/migration/qemu-file.c
> >> +++ b/migration/qemu-file.c
> >> @@ -595,7 +595,7 @@ size_t qemu_get_buffer_in_place(QEMUFile *f, uint8_t **buf, size_t size)
> >>  {
> >>      if (size < IO_BUF_SIZE) {
> >>          size_t res;
> >> -        uint8_t *src;
> >> +        uint8_t *src = NULL;
> >>  
> >>          res = qemu_peek_buffer(f, &src, size, 0);
> >>  
> >> -- 
> >> 2.28.0
> >>
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH] migration/qemu-file: Fix maybe uninitialized on qemu_get_buffer_in_place()

Posted by Dr. David Alan Gilbert 3 years, 2 months ago

* Dr. David Alan Gilbert (dgilbert@redhat.com) wrote:
> * Wainer dos Santos Moschetta (wainersm@redhat.com) wrote:
> > Fixed error when compiling migration/qemu-file.c with -Werror=maybe-uninitialized
> > as shown here:
> > 
> > ../migration/qemu-file.c: In function 'qemu_get_buffer_in_place':
> > ../migration/qemu-file.c:604:18: error: 'src' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> >   604 |             *buf = src;
> >       |             ~~~~~^~~~~
> > cc1: all warnings being treated as errors
> > 
> > Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
> 
> Yes, I think I had a discussion with someone about this recently but
> can't find it; the compiler is technically correct, but the only time
> it's unitialised is the case where it's result doesn't matter.
> 
> Still, to shut the compiler up:
> 
> 
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Queued

> 
> > ---
> > Passed on CI: https://gitlab.com/wainersm/qemu/-/pipelines/247801576
> > 
> >  migration/qemu-file.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/migration/qemu-file.c b/migration/qemu-file.c
> > index be21518c57..d6e03dbc0e 100644
> > --- a/migration/qemu-file.c
> > +++ b/migration/qemu-file.c
> > @@ -595,7 +595,7 @@ size_t qemu_get_buffer_in_place(QEMUFile *f, uint8_t **buf, size_t size)
> >  {
> >      if (size < IO_BUF_SIZE) {
> >          size_t res;
> > -        uint8_t *src;
> > +        uint8_t *src = NULL;
> >  
> >          res = qemu_peek_buffer(f, &src, size, 0);
> >  
> > -- 
> > 2.28.0
> > 
> -- 
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> 
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK