Re: [Qemu-devel] [PATCH 31/88] QMP: use g_new() family of functions

[Eric Blake]

On 10/09/2017 03:11 AM, Dr. David Alan Gilbert wrote:

>>>  
>>> -    info = g_malloc0(sizeof(*info));
>>> +    info = g_new0(CommandInfoList, 1);
>>>      info->value = g_malloc0(sizeof(*info->value));
>>>      info->value->name = g_strdup(cmd->name);
>>>      info->next = *list;
>>
>> I'm not convinced rewriting
>>
>>        LHS = g_malloc(sizeof(*LHS));
>>
>> to
>>
>>        LHS = g_new(T, 1);
>>
>> where T is the type of LHS is worth the trouble.  The code before the
>> rewrite is pretty idiomatic.  There's no possibility of integer overflow
>> g_new() could avoid.  The types are obviously correct, so the additional
>> type checking is quite unlikely to catch anything.  That leaves the
>> consistency argument.  I'm willing to hear it, but I feel it should be
>> heard in a patch series that does nothing else.
> 
> The 'obviously correct' is the dodgy part of the argument here.
> How many bugs do we right that are obviously wrong?
> 
> t.c:13:20: warning: initialization from incompatible pointer type 
> [-Wincompatible-pointer-types]
>      struct c *pc = g_new(struct b, 1);
> 
> seems good to me.

Yes, that's a GOOD warning, and it proves that we DO get type safety
when we convert:

LHS = g_malloc(sizeof(type))

into

LHS = g_new(type, 1)

but that's not what Markus was pointing out.  When we already have the
correctly-typed object on the right hand side, as in:

LHS = g_malloc(sizeof(*LHS))

then the compiler will always give us the correct type of LHS, whereas with:

LHS = g_new(type, 1)

we have to manually update the line if the type of LHS changes.

Thus, converting malloc(sizeof(type)) into new(type, 1) is a no-brainer,
but converting malloc(sizeof(*expr)) needs justification.  I'm not
necessarily opposed to the conversion (if our justification is
consistency, and where HACKING documents our style and where we have
scripts that let us easily preserve our style), but I'm not sure I see
the requisite justification in the current iteration of this series.

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

signature.asc
Description: OpenPGP digital signature