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

[Eric Blake <eblake@redhat.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