[PATCH for-5.2?] nbd: Silence Coverity false positive

[PATCH for-5.2?] nbd: Silence Coverity false positive

[Eric Blake]

Coverity noticed (CID 1436125) that we check the return value of
nbd_extent_array_add in most places, but not at the end of
bitmap_to_extents().  The return value exists to break loops before a
future iteration, so there is nothing to check if we are already done
iterating.  That said, a minor rearrangement to the code plus a better
comment pacifies Coverity.

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

Not a show-stopper by itself for 5.2, but perhaps worth including
in -rc2 if I collect other more-important patches.

 nbd/server.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/nbd/server.c b/nbd/server.c
index d145e1a69083..377698a2ce85 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -2128,9 +2128,8 @@ static void bitmap_to_extents(BdrvDirtyBitmap *bitmap,
         }
     }

-    if (!full) {
-        /* last non dirty extent */
-        nbd_extent_array_add(es, end - start, 0);
+    if (!full && nbd_extent_array_add(es, end - start, 0) < 0) {
+        /* last non dirty extent, nothing to do if array was already full */
     }

     bdrv_dirty_bitmap_unlock(bitmap);
-- 
2.28.0

Re: [PATCH for-5.2?] nbd: Silence Coverity false positive

[Richard Henderson]

On 11/11/20 8:35 AM, Eric Blake wrote:
> -    if (!full) {
> -        /* last non dirty extent */
> -        nbd_extent_array_add(es, end - start, 0);
> +    if (!full && nbd_extent_array_add(es, end - start, 0) < 0) {
> +        /* last non dirty extent, nothing to do if array was already full */
>      }

Casting to (void) is another way to get rid of the warning.

I dunno which makes more sense here.  Definitely the comment is helpful.

r~

Re: [PATCH for-5.2?] nbd: Silence Coverity false positive

[Eric Blake]

On 11/12/20 3:04 PM, Richard Henderson wrote:
> On 11/11/20 8:35 AM, Eric Blake wrote:
>> -    if (!full) {
>> -        /* last non dirty extent */
>> -        nbd_extent_array_add(es, end - start, 0);
>> +    if (!full && nbd_extent_array_add(es, end - start, 0) < 0) {
>> +        /* last non dirty extent, nothing to do if array was already full */
>>      }
> 
> Casting to (void) is another way to get rid of the warning.
> 
> I dunno which makes more sense here.  Definitely the comment is helpful.

As in:

if (!full) {
    /* last non dirty extent, nothing to do if array is now full */
    (void) nbd_extent_array_add(es, end - start, 0);
}

Yeah, that looks a little better.  Should I post that as v2, or wait for
further comments on this?

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

Re: [PATCH for-5.2?] nbd: Silence Coverity false positive

[Richard Henderson]

On 11/12/20 1:09 PM, Eric Blake wrote:
> On 11/12/20 3:04 PM, Richard Henderson wrote:
>> On 11/11/20 8:35 AM, Eric Blake wrote:
>>> -    if (!full) {
>>> -        /* last non dirty extent */
>>> -        nbd_extent_array_add(es, end - start, 0);
>>> +    if (!full && nbd_extent_array_add(es, end - start, 0) < 0) {
>>> +        /* last non dirty extent, nothing to do if array was already full */
>>>      }
>>
>> Casting to (void) is another way to get rid of the warning.
>>
>> I dunno which makes more sense here.  Definitely the comment is helpful.
> 
> As in:
> 
> if (!full) {
>     /* last non dirty extent, nothing to do if array is now full */
>     (void) nbd_extent_array_add(es, end - start, 0);
> }

Yep.

> Yeah, that looks a little better.  Should I post that as v2, or wait for
> further comments on this?

Up to you.  You can have my
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
for either version.


r~