[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Eric Blake
Subject: Re: [Qemu-trivial] [Qemu-devel] [PATCH 31/88] QMP: use g_new() family of functions
Date: Mon, 9 Oct 2017 13:04:35 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0

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))


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

Attachment: signature.asc
Description: OpenPGP digital signature

reply via email to

[Prev in Thread] Current Thread [Next in Thread]