bdrv_attach_child_common() doesn't require tran_finalize() on failure
(it does tran_add() only on success path). Still tran_new() must be
paired with tran_finalize() anyway, at least to free empty Transaction
object itself.

So, refactor the function for clean finalization code, same on all
paths.

While being here, also leave a comment on unobvious background zeroing
of child pointer on failure path.

Reported-by: Coverity (CID 1452773)
Reported-by: Peter Maydell <peter.mayd...@linaro.org>
Fixes: 548a74c0dbc858edd1a7ee3045b5f2fe710bd8b1
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
---
 block.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/block.c b/block.c
index 874c22c43e..728aa34b2f 100644
--- a/block.c
+++ b/block.c
@@ -2918,12 +2918,18 @@ BdrvChild *bdrv_root_attach_child(BlockDriverState 
*child_bs,
                                    child_role, perm, shared_perm, opaque,
                                    &child, tran, errp);
     if (ret < 0) {
-        bdrv_unref(child_bs);
-        return NULL;
+        goto out;
     }
 
     ret = bdrv_refresh_perms(child_bs, errp);
+    if (ret < 0) {
+        goto out;
+    }
+
+out:
     tran_finalize(tran, ret);
+    /* child is unset on failure by bdrv_attach_child_common_abort() */
+    assert((ret < 0) == !child);
 
     bdrv_unref(child_bs);
     return child;
-- 
2.29.2


Reply via email to