Re: [Qemu-devel] [PATCH] Introduce QEMU_NEW()

Stefan Hajnoczi
2011-07-25T10:28:34Z

On Mon, Jul 25, 2011 at 11:25 AM, Kevin Wolf <kwolf@redhat.com> wrote:
> Am 25.07.2011 12:06, schrieb Stefan Hajnoczi:
>> On Mon, Jul 25, 2011 at 9:51 AM, Avi Kivity <avi@redhat.com> wrote:
>>> qemu_malloc() is type-unsafe as it returns a void pointer.  Introduce
>>> QEMU_NEW() (and QEMU_NEWZ()), which return the correct type.
>>>
>>> Signed-off-by: Avi Kivity <avi@redhat.com>
>>> ---
>>>
>>> This is part of my memory API patchset, but doesn't really belong there.
>>>
>>>  qemu-common.h |    3 +++
>>>  1 files changed, 3 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/qemu-common.h b/qemu-common.h
>>> index ba55719..66effa3 100644
>>> --- a/qemu-common.h
>>> +++ b/qemu-common.h
>>> @@ -186,6 +186,9 @@ void qemu_free(void *ptr);
>>>  char *qemu_strdup(const char *str);
>>>  char *qemu_strndup(const char *str, size_t size);
>>>
>>> +#define QEMU_NEW(type) ((type *)(qemu_malloc(sizeof(type))))
>>> +#define QEMU_NEWZ(type) ((type *)(qemu_mallocz(sizeof(type))))
>>
>> Does this mean we need to duplicate the type name for each allocation?
>>
>> struct foo *f;
>>
>> ...
>> f = qemu_malloc(sizeof(*f));
>>
>> Becomes:
>>
>> struct foo *f;
>>
>> ...
>> f = QEMU_NEW(struct foo);
>
> Maybe we should allow this and make it the usual pattern:
>
> f = qemu_new(typeof(*f));
>
> It's gcc specific, but we already don't care about portability to other
> compilers in more places.
>
> On the other hand, how many bugs did we have recently that were caused
> by a wrong sizeof for qemu_malloc? As far as I can say, there's no real
> reason to do it. I think it's the same kind of discussion as with
> forbidding qemu_malloc(0) (except that this time it just won't improve
> things much instead of being really stupid).

Totally agree. In theory you can add stuff on top to prevent known
bad uses but in practice it's not worth obfuscating the code.

Stefan