Fix warning reported by Clang static code analyzer:

  block.c:3167:5: warning: Value stored to 'ret' is never read
      ret = bdrv_fill_options(&options, filename, &flags, &local_err);
      ^     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Reported-by: Clang Static Analyzer
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 block.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block.c b/block.c
index a2542c977b..908c109a8c 100644
--- a/block.c
+++ b/block.c
@@ -3164,7 +3164,7 @@ static BlockDriverState *bdrv_open_inherit(const char *filename,
                                     parent->open_flags, parent->options);
     }
 
-    ret = bdrv_fill_options(&options, filename, &flags, &local_err);
+    bdrv_fill_options(&options, filename, &flags, &local_err);
     if (local_err) {
         goto fail;
     }
-- 
2.21.1

12:49 PM Sub, 21.03.2020. Philippe Mathieu-Daudé <philmd@redhat.com> је
написао/ла:
>
> Fix warning reported by Clang static code analyzer:
>
>   block.c:3167:5: warning: Value stored to 'ret' is never read
>       ret = bdrv_fill_options(&options, filename, &flags, &local_err);
>       ^     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Reported-by: Clang Static Analyzer

Peter, and others,

Is this allowed use of "Reported-by:" mark?

I did not notice it being used this way before. And was under impression
that all similar tags/marks must be followed by a person, not a tool.

Regards,
Aleksandar

> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  block.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/block.c b/block.c
> index a2542c977b..908c109a8c 100644
> --- a/block.c
> +++ b/block.c
> @@ -3164,7 +3164,7 @@ static BlockDriverState *bdrv_open_inherit(const
char *filename,
>                                      parent->open_flags, parent->options);
>      }
>
> -    ret = bdrv_fill_options(&options, filename, &flags, &local_err);
> +    bdrv_fill_options(&options, filename, &flags, &local_err);
>      if (local_err) {
>          goto fail;
>      }
> --
> 2.21.1
>
>

On Sat, 21 Mar 2020, Aleksandar Markovic wrote:
> 12:49 PM Sub, 21.03.2020. Philippe Mathieu-Daudé <philmd@redhat.com> је
> написао/ла:
>>
>> Fix warning reported by Clang static code analyzer:
>>
>>   block.c:3167:5: warning: Value stored to 'ret' is never read
>>       ret = bdrv_fill_options(&options, filename, &flags, &local_err);
>>       ^     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> Reported-by: Clang Static Analyzer
>
> Peter, and others,
>
> Is this allowed use of "Reported-by:" mark?
>
> I did not notice it being used this way before. And was under impression
> that all similar tags/marks must be followed by a person, not a tool.

I think there were previous patches with Reported-by: Euler Robot which 
may be similar to this usage.

Regards,
BALATON Zoltan

>
> Regards,
> Aleksandar
>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>>  block.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/block.c b/block.c
>> index a2542c977b..908c109a8c 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -3164,7 +3164,7 @@ static BlockDriverState *bdrv_open_inherit(const
> char *filename,
>>                                      parent->open_flags, parent->options);
>>      }
>>
>> -    ret = bdrv_fill_options(&options, filename, &flags, &local_err);
>> +    bdrv_fill_options(&options, filename, &flags, &local_err);
>>      if (local_err) {
>>          goto fail;
>>      }
>> --
>> 2.21.1
>>
>>
>

Le 21/03/2020 à 12:46, Philippe Mathieu-Daudé a écrit :
> Fix warning reported by Clang static code analyzer:
> 
>   block.c:3167:5: warning: Value stored to 'ret' is never read
>       ret = bdrv_fill_options(&options, filename, &flags, &local_err);
>       ^     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Reported-by: Clang Static Analyzer
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  block.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/block.c b/block.c
> index a2542c977b..908c109a8c 100644
> --- a/block.c
> +++ b/block.c
> @@ -3164,7 +3164,7 @@ static BlockDriverState *bdrv_open_inherit(const char *filename,
>                                      parent->open_flags, parent->options);
>      }
>  
> -    ret = bdrv_fill_options(&options, filename, &flags, &local_err);
> +    bdrv_fill_options(&options, filename, &flags, &local_err);
>      if (local_err) {
>          goto fail;
>      }
> 

I would be sruprised if coverity doesn't warn about an unused return value.

Thanks,
Laurent

Laurent Vivier <laurent@vivier.eu> writes:

> Le 21/03/2020 à 12:46, Philippe Mathieu-Daudé a écrit :
>> Fix warning reported by Clang static code analyzer:
>> 
>>   block.c:3167:5: warning: Value stored to 'ret' is never read
>>       ret = bdrv_fill_options(&options, filename, &flags, &local_err);
>>       ^     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> 
>> Reported-by: Clang Static Analyzer
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>>  block.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/block.c b/block.c
>> index a2542c977b..908c109a8c 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -3164,7 +3164,7 @@ static BlockDriverState *bdrv_open_inherit(const char *filename,
>>                                      parent->open_flags, parent->options);
>>      }
>>  
>> -    ret = bdrv_fill_options(&options, filename, &flags, &local_err);
>> +    bdrv_fill_options(&options, filename, &flags, &local_err);
>>      if (local_err) {
>>          goto fail;
>>      }
>> 
>
> I would be sruprised if coverity doesn't warn about an unused return value.

Coverity recognizes the fact that some return values can be safely
ignored, and reports only ignored return values it sees commonly checked
elsewhere.

This function is used called nowhere else.  Coverity won't complain.

However, I'd prefer

        ret = bdrv_fill_options(&options, filename, &flags, &local_err);
   -    if (local_err) {
   +    if (ret < 0) {
            goto fail;
        }