Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() & friends

[Avi Kivity <avi@redhat.com>]

On 12/05/2009 07:27 PM, Anthony Liguori wrote:
> Avi Kivity wrote:
>> On 12/04/2009 06:49 PM, Anthony Liguori wrote:
>>>
>>> I still believe that it is poor practice to pass size==0 to 
>>> *malloc().  I think actively discouraging this in qemu is a good 
>>> thing because it's a broken idiom.
>>
>> Why?  Unless we have a separate array allocator (like C++'s new and 
>> new[]), we need to support zero-element arrays without pushing the 
>> burden to callers (in the same way that for () supports zero 
>> iteration loops without a separate if ()).
>
> If you call malloc(size=0), then it's impossible to differentiate 
> between a failed malloc return and a valid result on certain platform 
> (like AIX).
>
> So the only correct way would be to write:
>
> array = malloc(size);
> if (array == NULL && size != 0) {
>    return -ENOMEM;
> }
>
> If you were writing portable code.  But that's not what people write.  
> You can argue that qemu_malloc() can have any semantics we want and 
> while that's true, it doesn't change my above statement.  I think the 
> main argument for this behavior in qemu is that people are used to 
> using this idiom with malloc() but it's a non-portable practice.
>
> If qemu_malloc() didn't carry the name "malloc()" then semantics with 
> size=0 would be a different discussion.  But so far, all qemu_* 
> functions tend to behave almost exactly like their C counterparts.  
> Relying on the result of size=0 with malloc() is broken.

A zero-supporting qemu_malloc() is fully compatible with malloc(), we're 
only restricting the possible returns.  So we're not misleading any 
caller.  In fact, taking your argument to the extreme, a malloc 
implementation would need to

    if (size == 0) {
       if (rand() % 2) {
           return NULL;
       } else {
          size = 1;
       }
    }

Note that qemu_malloc() already restricts return values by not returning 
NULL on oom, according to your logic we shouldn't do this (and have two 
tests accompany any variable-size qemu_malloc()).

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.