Re: [Qemu-devel] [PATCH] use qemu_malloc and friends consistently

[malc <av1474@comtv.ru>]

On Fri, 29 May 2009, Anthony Liguori wrote:

> malc wrote:
> > On Fri, 29 May 2009, Anthony Liguori wrote:
> >
> >
> > Dereference of NULL is UB[1] and dereferencing result of malloc(1) will
> > just plain work.
> >
>
> So let's ignoring returning NULL as a possibility..
>
> >> Putting the abort() in there is going to introduce a ton of subtle bugs,
> >> I vote for changing qemu_malloc() to have a sane behavior.
> >>
> >
> > And those will be caught, given one a chance to analyze things, unlike
> > head in the sand approach of hoping things would just work.
> >
> > After doing some research, after the aforementioned lengthy discussion,
> > the only free OS that straight-forwardly described what it does was
> > OpenBSD:
> >
> > http://www.openbsd.org/cgi-bin/man.cgi?query=malloc&apropos=0&sektion=0&manpath=OpenBSD+Current&arch=i386&format=html
> >
> > P.S. So far the abort that went into qemu_malloc caught one usage of zero
> >      allocation (once again coming from qcow2).'
> >
>
> But the zero allocation isn't a bug if we return malloc(1).  This is a
> common convention and while it may not be portable to every platform's
> underlying malloc, we can make this convention portable with qemu_malloc().

Yes we can. I argue that this is not a good convention.

>
> At the end of the day, the result is a harder to misuse qemu_malloc()
> and that's a very good thing.  I don't want a user to "discover" a
> non-portable use of malloc() while trying to do something important.

Options:

a. return NULL
b. return malloc(1)
c. abort
d. do what OpenBSD does

Pros/cons:

a. Pros: Simple to implement
         Matches one of original malloc behaviours

   Cons: Observable
         Dereference == UB
         Breaks the assumption that qemu_malloc should never return NULL


b. Pros: Simple to implement
         Matches one of original malloc behaviours

   Cons: Useless allocation
         No guard against accidental dereferences
         Might result in a return of a NULL


c. Pros: Simple to implement
         Helps in finding call-sites

   Cons: Doesn't match match the standard prescribed behaviours
         Will abort the application even if the call-site is prepared
         to cope with the fact that it requested zero bytes


d. Pros: Matches one of original malloc behaviours
         Provides safety net against accidental dereferences

   Cons: Not trivial to implement (and also complicates the whole family
         qemu_realloc/qemu_free)
         Doesn't help in identifying call-sites


In a nutshell what i argue is that, if someone doesn't need any memory
it shouldn't be asking for it, and it's not that unlikely that the
author never considered the possibility of his code requesting zero
bytes of memory, so in my view helping to locate all offenders and
auditing them is good enough reason for option c, that's why it's
there in repository.

-- 
mailto:av1474@comtv.ru