Re: [PATCH-for-5.0 01/11] block: Remove dead assignment

[Markus Armbruster]

Laurent Vivier <address@hidden> 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é <address@hidden>
>> ---
>>  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;
        }