Re: [RFC PATCH 1/4] block/vdi: When writing new bmap entry fails, don't leak the buffer

[David Edmondson <dme@dme.org>]

On Tuesday, 2021-03-09 at 12:09:55 +01, Philippe Mathieu-Daudé wrote:

> On 3/9/21 11:21 AM, David Edmondson wrote:
>> If a new bitmap entry is allocated, requiring the entire block to be
>> written, avoiding leaking the buffer allocated for the block should
>> the write fail.
>> 
>> Signed-off-by: David Edmondson <david.edmondson@oracle.com>
>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Thanks.

>> ---
>>  block/vdi.c | 1 +
>>  1 file changed, 1 insertion(+)
>> 
>> diff --git a/block/vdi.c b/block/vdi.c
>> index 5627e7d764..2a6dc26124 100644
>> --- a/block/vdi.c
>> +++ b/block/vdi.c
>> @@ -690,6 +690,7 @@ nonallocating_write:
>>  
>>      logout("finished data write\n");
>>      if (ret < 0) {
>> +        g_free(block);
>>          return ret;
>>      }
>
> Alternative using g_autofree:

Newfangled witchy magic!

I'm happy to change it if you think it beneficial.

> -- >8 --
> diff --git a/block/vdi.c b/block/vdi.c
> index 5627e7d764a..1cd8ae2ba99 100644
> --- a/block/vdi.c
> +++ b/block/vdi.c
> @@ -612,7 +612,7 @@ vdi_co_pwritev(BlockDriverState *bs, uint64_t
> offset, uint64_t bytes,
>      uint64_t data_offset;
>      uint32_t bmap_first = VDI_UNALLOCATED;
>      uint32_t bmap_last = VDI_UNALLOCATED;
> -    uint8_t *block = NULL;
> +    g_autofree uint8_t *block = NULL;
>      uint64_t bytes_done = 0;
>      int ret = 0;
>
> @@ -705,9 +705,6 @@ nonallocating_write:
>          *header = s->header;
>          vdi_header_to_le(header);
>          ret = bdrv_pwrite(bs->file, 0, block, sizeof(VdiHeader));
> -        g_free(block);
> -        block = NULL;
> -
>          if (ret < 0) {
>              return ret;
>          }
> ---

dme.
-- 
Tonight I think I'll walk alone, I'll find my soul as I go home.