Re: [Qemu-devel] [PATCH] fix qemu_malloc() error check for size==0

[Eduardo Habkost]

On Tue, May 19, 2009 at 06:37:18PM +0400, malc wrote:
> On Tue, 19 May 2009, Eduardo Habkost wrote:
> 
> > On Tue, May 19, 2009 at 05:00:27PM +0400, malc wrote:
> > > On Tue, 19 May 2009, Markus Armbruster wrote:
> > <snip>
> > > > >                                              IOW making qemu_malloc[z]
> > > > > return whatever the underlying system returns is just hiding the bugs,
> > > > > the code becomes unportable.
> > > > 
> > > > Matter of taste.
> > > > 
> > > > 1. Deal with the implementation-definedness.  Every caller that could
> > > >    pass zero needs to take care not to confuse empty allocation with an
> > > >    out of memory condition.
> > > > 
> > > >    This is easier than it sounds when you check for out of memory in
> > > >    just one place, like we do.
> > > > 
> > > > 2. Remove the implementation-definedness.  Easiest way is to detect zero
> > > >    size in a wrapper (for us: qemu_malloc()) and bump it to one.
> > > 
> > > And mine:
> > >   3. Abort the program if somebody tries it. Because so far history 
> > > thought
> > >      me that nobody does 1.
> > 
> > Are you sure about that? There may be cases where qemu_malloc(0) is
> > called correctly, without the wrong assumptions about the returned
> > value.
> > 
> > You are proposing to make the qemu_malloc() API behavior diverge from
> > the standard C malloc() behavior and prevent usage that is valid for
> > malloc()/free() usage. Do you volunteer to audit all Qemu code to make
> > sure the new behavior is safe?  ;)
> 
> That's the problem standard C does _not_ define the behaviour, and leaves
> that to implementation.

The only thing it doesn't define is either the returned pointer is NULL
or not, and that doesn't make malloc(0) automatically unportable,
because all the rest is perfectly defined:

1) You can't dereference the pointer (just like you can't
   dereference p[n] on a malloc(n) block)
2) You should pass the returned pointer to free() later


> As for audit, that's precisely what aborting on
> zero will try (and fail) to accomplish the offenders will be (given
> unlimited time) caught.

My point is that malloc(0) is not a bug, and I don't see a reason to
make it an offense and diverge from standard malloc() and free().

> oom_check was added despite the fact that there
> were places that correctly handled malloc's returning NULL. And i for
> one do not know if there are/were places that called qemu_malloc with
> zero and expected Linux behaviour.

I agree that expecting the Linux behaviour (non-NULL) is a bug. My point
is that there is no reason to consider malloc(0) a bug.

-- 
Eduardo