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

[Vladimir Sementsov-Ogievskiy]

05.02.2021 14:43, Alberto Garcia wrote:
> 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.

No. *info_list is modified only on the first loop iteration. And than info_list is 
switched to &(*(info_list))->next, so on second iteration we will modify @next 
field of first element, not original *info_list.



--
Best regards,
Vladimir