Re: [PATCH v7 08/14] block/qcow2: qcow2_get_specific_info(): drop error propagation

[Alberto Garcia]

On Tue 02 Feb 2021 01:49:50 PM CET, Vladimir Sementsov-Ogievskiy wrote:
> -Qcow2BitmapInfoList *qcow2_get_bitmap_info_list(BlockDriverState *bs,
> -                                                Error **errp)
> +bool qcow2_get_bitmap_info_list(BlockDriverState *bs,
> +                                Qcow2BitmapInfoList **info_list, Error 
> **errp)
>  {
>      BDRVQcow2State *s = bs->opaque;
>      Qcow2BitmapList *bm_list;
>      Qcow2Bitmap *bm;
> -    Qcow2BitmapInfoList *list = NULL;
> -    Qcow2BitmapInfoList **tail = &list;
>  
>      if (s->nb_bitmaps == 0) {
> -        return NULL;
> +        *info_list = NULL;
> +        return true;
>      }
>  
>      bm_list = bitmap_list_load(bs, s->bitmap_directory_offset,
>                                 s->bitmap_directory_size, errp);
> -    if (bm_list == NULL) {
> -        return NULL;
> +    if (!bm_list) {
> +        return false;
>      }
>  
> +    *info_list = NULL;
> +
>      QSIMPLEQ_FOREACH(bm, bm_list, entry) {
>          Qcow2BitmapInfo *info = g_new0(Qcow2BitmapInfo, 1);
>          info->granularity = 1U << bm->granularity_bits;
>          info->name = g_strdup(bm->name);
>          info->flags = get_bitmap_info_flags(bm->flags & ~BME_RESERVED_FLAGS);
> -        QAPI_LIST_APPEND(tail, info);
> +        QAPI_LIST_APPEND(info_list, info);
>      }
>  
>      bitmap_list_free(bm_list);
>  
> -    return list;
> +    return true;
>  }

Maybe I'm reading this wrong but...

In the original code you had the head and tail of the list ('list' and
'tail') then you would append items to the tail and finally return the
head.

However the new code only uses and updates 'info_list' and it does not
keep the head anywhere, so what the caller gets is a pointer to the
tail.

Berto