diff mbox series

[for-4.1] block: Assert that drv->bdrv_child_perm is set in bdrv_child_perm()

Message ID 20190404112953.4058-1-berto@igalia.com (mailing list archive)
State New, archived
Headers show
Series [for-4.1] block: Assert that drv->bdrv_child_perm is set in bdrv_child_perm() | expand

Commit Message

Alberto Garcia April 4, 2019, 11:29 a.m. UTC
There is no need to check for this because all block drivers that have
children implement bdrv_child_perm and all callers already ensure that
bs->drv is set.

Furthermore, if this check would fail then the callers would end up
with uninitialized values for nperm and nshared.

This patch replaces the check with an assertion.

Signed-off-by: Alberto Garcia <berto@igalia.com>
---
 block.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

Comments

Alberto Garcia April 26, 2019, 2:01 p.m. UTC | #1
ping

On Thu 04 Apr 2019 01:29:53 PM CEST, Alberto Garcia wrote:
> There is no need to check for this because all block drivers that have
> children implement bdrv_child_perm and all callers already ensure that
> bs->drv is set.
>
> Furthermore, if this check would fail then the callers would end up
> with uninitialized values for nperm and nshared.
>
> This patch replaces the check with an assertion.
>
> Signed-off-by: Alberto Garcia <berto@igalia.com>
> ---
>  block.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/block.c b/block.c
> index 3050854528..5f92565692 100644
> --- a/block.c
> +++ b/block.c
> @@ -1742,11 +1742,10 @@ static void bdrv_child_perm(BlockDriverState *bs, BlockDriverState *child_bs,
>                              uint64_t parent_perm, uint64_t parent_shared,
>                              uint64_t *nperm, uint64_t *nshared)
>  {
> -    if (bs->drv && bs->drv->bdrv_child_perm) {
> -        bs->drv->bdrv_child_perm(bs, c, role, reopen_queue,
> -                                 parent_perm, parent_shared,
> -                                 nperm, nshared);
> -    }
> +    assert(bs->drv && bs->drv->bdrv_child_perm);
> +    bs->drv->bdrv_child_perm(bs, c, role, reopen_queue,
> +                             parent_perm, parent_shared,
> +                             nperm, nshared);
>      /* TODO Take force_share from reopen_queue */
>      if (child_bs && child_bs->force_share) {
>          *nshared = BLK_PERM_ALL;
> -- 
> 2.11.0
Max Reitz April 26, 2019, 9:09 p.m. UTC | #2
On 04.04.19 13:29, Alberto Garcia wrote:
> There is no need to check for this because all block drivers that have
> children implement bdrv_child_perm and all callers already ensure that
> bs->drv is set.
> 
> Furthermore, if this check would fail then the callers would end up
> with uninitialized values for nperm and nshared.
> 
> This patch replaces the check with an assertion.
> 
> Signed-off-by: Alberto Garcia <berto@igalia.com>
> ---
>  block.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)

Thanks, applied to my block branch:

https://git.xanclic.moe/XanClic/qemu/commits/branch/block

Max
diff mbox series

Patch

diff --git a/block.c b/block.c
index 3050854528..5f92565692 100644
--- a/block.c
+++ b/block.c
@@ -1742,11 +1742,10 @@  static void bdrv_child_perm(BlockDriverState *bs, BlockDriverState *child_bs,
                             uint64_t parent_perm, uint64_t parent_shared,
                             uint64_t *nperm, uint64_t *nshared)
 {
-    if (bs->drv && bs->drv->bdrv_child_perm) {
-        bs->drv->bdrv_child_perm(bs, c, role, reopen_queue,
-                                 parent_perm, parent_shared,
-                                 nperm, nshared);
-    }
+    assert(bs->drv && bs->drv->bdrv_child_perm);
+    bs->drv->bdrv_child_perm(bs, c, role, reopen_queue,
+                             parent_perm, parent_shared,
+                             nperm, nshared);
     /* TODO Take force_share from reopen_queue */
     if (child_bs && child_bs->force_share) {
         *nshared = BLK_PERM_ALL;