On 30.03.22 23:28, Vladimir Sementsov-Ogievskiy wrote:
Now the indirection is not actually used, we can safely reduce it to
simple pointer.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@openvz.org>
---
block/snapshot.c | 39 +++++++++++++++++----------------------
1 file changed, 17 insertions(+), 22 deletions(-)
Looks good, just wondering whether we should drop some of the "_ptr" suffixes
now.
diff --git a/block/snapshot.c b/block/snapshot.c
index 02a880911f..4eb9258de6 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -151,34 +151,29 @@ bool bdrv_snapshot_find_by_id_and_name(BlockDriverState
*bs,
}
/**
- * Return a pointer to the child BDS pointer to which we can fall
+ * Return a pointer to child of given BDS to which we can fall
* back if the given BDS does not support snapshots.
* Return NULL if there is no BDS to (safely) fall back to.
- *
- * We need to return an indirect pointer because bdrv_snapshot_goto()
- * has to modify the BdrvChild pointer.
*/
-static BdrvChild **bdrv_snapshot_fallback_ptr(BlockDriverState *bs)
+static BdrvChild *bdrv_snapshot_fallback_ptr(BlockDriverState *bs)
The _ptr part was meant to point out that it returns an indirect pointer; maybe
we should name it bdrv_snapshot_fallback_child() now?
{
- BdrvChild **fallback;
- BdrvChild *child = bdrv_primary_child(bs);
+ BdrvChild *fallback = bdrv_primary_child(bs);
+ BdrvChild *child;
/* We allow fallback only to primary child */
- if (!child) {
+ if (!fallback) {
return NULL;
}
- fallback = (child == bs->file ? &bs->file : &bs->backing);
- assert(*fallback == child);
/*
* Check that there are no other children that would need to be
* snapshotted. If there are, it is not safe to fall back to
- * *fallback.
+ * fallback.
*/
QLIST_FOREACH(child, &bs->children, next) {
if (child->role & (BDRV_CHILD_DATA | BDRV_CHILD_METADATA |
BDRV_CHILD_FILTERED) &&
- child != *fallback)
+ child != fallback)
{
return NULL;
}
@@ -189,8 +184,8 @@ static BdrvChild
**bdrv_snapshot_fallback_ptr(BlockDriverState *bs)
static BlockDriverState *bdrv_snapshot_fallback(BlockDriverState *bs)
{
- BdrvChild **child_ptr = bdrv_snapshot_fallback_ptr(bs);
Just "child" is enough (and better) now, I think.
- return child_ptr ? (*child_ptr)->bs : NULL;
+ BdrvChild *child_ptr = bdrv_snapshot_fallback_ptr(bs);
+ return child_ptr ? child_ptr->bs : NULL;
}
int bdrv_can_snapshot(BlockDriverState *bs)