Re: nbd: bitmap_to_extents() calls nbd_extent_array_add() without checking return value: coverity false positive?

[Peter Maydell <peter.maydell@linaro.org>]

On Fri, 6 Nov 2020 at 20:36, Eric Blake <eblake@redhat.com> wrote:
>
> On 11/6/20 11:22 AM, Peter Maydell wrote:
> > Hi; Coverity's "you usually check the return value of this function
> > but you didn't do that here" heuristic has fired on the code in
> > nbd/server.c:bitmap_to_extents() -- the function nbd_extent_array_add()
> > is called five times in server.c, and the return value is checked
> > in four of those, but not in the final call at the end of
> > bitmap_to_extents(). (CID 1436125.)
> >
> > Is this a false positive, or should the caller be handling an
> > error here ?
>
> False positive, but I don't mind tweaking the code to silence Coverity.
> This should do it; let me know if I should turn it into a formal patch.
>
> diff --git i/nbd/server.c w/nbd/server.c
> index d145e1a69083..377698a2ce85 100644
> --- i/nbd/server.c
> +++ w/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, not a problem if array is now full */
>      }
>
>      bdrv_dirty_bitmap_unlock(bitmap);

Hmm; that looks a little odd but I guess it's a bit more
documentative of the intent. Up to you whether you want
to submit it as a patch or not I guess :-)

thanks
-- PMM