Re: [Qemu-devel] [PATCH v2 09/10] qemu-img: Make img_convert() get image size just once per image

[Markus Armbruster]

Max Reitz <address@hidden> writes:

> On 28.05.2014 16:25, Markus Armbruster wrote:
>> Chiefly so I don't have to do the error checking in quadruplicate in
>> the next commit.  Moreover, replacing the frequently updated
>> bs_sectors by an array assigned just once makes the code easier to
>> understand.
>>
>> Signed-off-by: Markus Armbruster <address@hidden>
>> ---
>>   qemu-img.c | 32 ++++++++++++++++----------------
>>   1 file changed, 16 insertions(+), 16 deletions(-)
>>
>> diff --git a/qemu-img.c b/qemu-img.c
>> index 8d996ba..229c0c6 100644
>> --- a/qemu-img.c
>> +++ b/qemu-img.c
>> @@ -1186,7 +1186,7 @@ static int img_convert(int argc, char **argv)
>>       BlockDriver *drv, *proto_drv;
>>       BlockDriverState **bs = NULL, *out_bs = NULL;
>>       int64_t total_sectors, nb_sectors, sector_num, bs_offset;
>> -    uint64_t bs_sectors;
>> +    uint64_t *bs_sectors = NULL;
>>       uint8_t * buf = NULL;
>>       size_t bufsectors = IO_BUF_SIZE / BDRV_SECTOR_SIZE;
>>       const uint8_t *buf1;
>> @@ -1326,7 +1326,8 @@ static int img_convert(int argc, char **argv)
>>         qemu_progress_print(0, 100);
>>   -    bs = g_malloc0(bs_n * sizeof(BlockDriverState *));
>> +    bs = g_new(BlockDriverState *, bs_n);
>
> I think this should rather be g_new0(), so the clean-up code after
> "out:" doesn't get confused about which BDS are valid and which are
> not.

You're right.

I'm afraid manually converting to g_new() & friends when I touch the
line anyway is too error prone for me.  I'll dust off the block parts of
my "Use g_new() & friends where that makes obvious sense" series.

> Other than that, this patch looks good to me.

Thanks!  May I add your R-by if all I change is fixing the g_new() to
g_new0()?