[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v7 08/14] block/qcow2: qcow2_get_specific_info(): drop error
From: |
Eric Blake |
Subject: |
Re: [PATCH v7 08/14] block/qcow2: qcow2_get_specific_info(): drop error propagation |
Date: |
Fri, 12 Feb 2021 13:44:19 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.0 |
On 2/5/21 5:52 AM, Vladimir Sementsov-Ogievskiy wrote:
> 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.
Elsewhere when making these types of conversions, Markus suggested that
I keep a separate tail variable, initialized by the parameter info_list,
to make it more apparent. As in squashing the patch below:
With that, it looks this series is reviewed, so I'm planning on taking
it through my dirty-bitmaps tree (perhaps I'm stretching the fact that
patch 10/14 is definitely dirty-bitmaps into taking the whole series,
but I doubt I'll hear any complaints from other block maintainers)
diff --git i/block/qcow2-bitmap.c w/block/qcow2-bitmap.c
index e50da1ee7da3..f417f9ccb195 100644
--- i/block/qcow2-bitmap.c
+++ w/block/qcow2-bitmap.c
@@ -1103,6 +1103,7 @@ bool qcow2_get_bitmap_info_list(BlockDriverState *bs,
BDRVQcow2State *s = bs->opaque;
Qcow2BitmapList *bm_list;
Qcow2Bitmap *bm;
+ Qcow2BitmapInfoList **tail;
if (s->nb_bitmaps == 0) {
*info_list = NULL;
@@ -1116,13 +1117,14 @@ bool qcow2_get_bitmap_info_list(BlockDriverState
*bs,
}
*info_list = NULL;
+ tail = info_list;
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(info_list, info);
+ QAPI_LIST_APPEND(tail, info);
}
bitmap_list_free(bm_list);
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
- Re: [PATCH v7 03/14] block: check return value of bdrv_open_child and drop error propagation, (continued)
[PATCH v7 06/14] block/mirror: drop extra error propagation in commit_active_start(), Vladimir Sementsov-Ogievskiy, 2021/02/02
[PATCH v7 10/14] block/qcow2-bitmap: return status from qcow2_store_persistent_dirty_bitmaps, Vladimir Sementsov-Ogievskiy, 2021/02/02
[PATCH v7 08/14] block/qcow2: qcow2_get_specific_info(): drop error propagation, Vladimir Sementsov-Ogievskiy, 2021/02/02
[PATCH v7 07/14] blockjob: return status from block_job_set_speed(), Vladimir Sementsov-Ogievskiy, 2021/02/02
[PATCH v7 13/14] block/qed: bdrv_qed_do_open: deal with errp, Vladimir Sementsov-Ogievskiy, 2021/02/02
[PATCH v7 09/14] block/qcow2-bitmap: improve qcow2_load_dirty_bitmaps() interface, Vladimir Sementsov-Ogievskiy, 2021/02/02
[PATCH v7 14/14] block/qcow2: refactor qcow2_update_options_prepare error paths, Vladimir Sementsov-Ogievskiy, 2021/02/02
[PATCH v7 11/14] block/qcow2: read_cache_sizes: return status value, Vladimir Sementsov-Ogievskiy, 2021/02/02
[PATCH v7 12/14] block/qcow2: simplify qcow2_co_invalidate_cache(), Vladimir Sementsov-Ogievskiy, 2021/02/02