On Thu 17 Sep 2020 09:55:14 PM CEST, Vladimir Sementsov-Ogievskiy wrote:
> Don't use error propagation in qcow2_get_specific_info(). For this
> refactor qcow2_get_bitmap_info_list, its current interface is rather
> weird.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
> Reviewed-by: Greg Kurz <gr...@kaod.org>
> - * In case of no bitmaps, the function returns NULL and
> - * the @errp parameter is not set.
> - * When bitmap information can not be obtained, the function returns
> - * NULL and the @errp parameter is set.
> + * On success return true with bm_list set (probably to NULL, if no bitmaps),
> + * on failure return false with errp set.
I still find the 'probably' thing odd :-) I think the API documentation
should describe what the function does and under which conditions, not
the probability of the outcomes.
> QSIMPLEQ_FOREACH(bm, bm_list, entry) {
> Qcow2BitmapInfo *info = g_new0(Qcow2BitmapInfo, 1);
> Qcow2BitmapInfoList *obj = g_new0(Qcow2BitmapInfoList, 1);
> @@ -1124,13 +1123,13 @@ Qcow2BitmapInfoList
> *qcow2_get_bitmap_info_list(BlockDriverState *bs,
> info->name = g_strdup(bm->name);
> info->flags = get_bitmap_info_flags(bm->flags & ~BME_RESERVED_FLAGS);
> obj->value = info;
> - *plist = obj;
> - plist = &obj->next;
> + *info_list = obj;
> + info_list = &obj->next;
> }
You were right with this, I got confused by the fact that you are
modifying the pointer provided by the user.
Reviewed-by: Alberto Garcia <be...@igalia.com>
Berto
