Re: [PATCH 08/10] qcow2-refcount: improve style of check_refcounts_l1()

From: Eric Blake <eblake@redhat.com>
To: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>,
	qemu-block@nongnu.org
Cc: kwolf@redhat.com, den@openvz.org, ktkhai@virtuozzo.com,
	qemu-devel@nongnu.org, mreitz@redhat.com
Subject: Re: [PATCH 08/10] qcow2-refcount: improve style of check_refcounts_l1()
Date: Tue, 4 May 2021 14:53:24 -0500	[thread overview]
Message-ID: <d3b3564b-16c7-40d4-d229-363b77d2c0ee@redhat.com> (raw)
In-Reply-To: <20210504152023.322862-9-vsementsov@virtuozzo.com>

On 5/4/21 10:20 AM, Vladimir Sementsov-Ogievskiy wrote:
>  - use g_autofree for l1_table
>  - better name for size in bytes variable
>  - reduce code blocks nesting
>  - whitespaces, braces, newlines
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  block/qcow2-refcount.c | 97 +++++++++++++++++++++---------------------
>  1 file changed, 49 insertions(+), 48 deletions(-)
> 
> diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
> index 44fc0dd5dc..eb6de3dabd 100644
> --- a/block/qcow2-refcount.c
> +++ b/block/qcow2-refcount.c
> @@ -1864,71 +1864,72 @@ static int check_refcounts_l1(BlockDriverState *bs,
>                                int flags, BdrvCheckMode fix, bool active)
>  {
>      BDRVQcow2State *s = bs->opaque;
> -    uint64_t *l1_table = NULL, l2_offset, l1_size2;
> +    size_t l1_size_bytes = l1_size * L1E_SIZE;
> +    g_autofree uint64_t *l1_table = g_try_malloc(l1_size_bytes);

Note that this now happens...

> +    uint64_t l2_offset;
>      int i, ret;
>  
> -    l1_size2 = l1_size * L1E_SIZE;
> +    if (!l1_size) {
> +        return 0;

...before you validate whether l1_size is non-zero, which can result in
g_try_malloc(0).  Probably harmless, but it might be better if you declare
 g_autofree uint64_t *l1_table = NULL;
and then initialize it via malloc only after the sanity check.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org