[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] use qemu_malloc and friends consistently
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH] use qemu_malloc and friends consistently |
Date: |
Fri, 29 May 2009 14:51:39 +0200 |
User-agent: |
Gnus/5.11 (Gnus v5.11) Emacs/22.3 (gnu/linux) |
malc <address@hidden> writes:
> 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
Neat.
What happens when malloc(0) can't obtain space in "shared protected
pages"? I figure it fails and returns NULL.
>> > 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
Might result in a return of a NULL
> 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.
Do we want a drop-in replacement for malloc() or not?
If we do, then options a, b and d are fine[*]. c is not, because it
breaks working code. What the programmer thought when he wrote the
working code is immaterial.
[*] None of them guarantee that malloc(0) != NULL always, although b and
d make it unlikely.
- Re: [Qemu-devel] [PATCH] use qemu_malloc and friends consistently, (continued)
- Re: [Qemu-devel] [PATCH] use qemu_malloc and friends consistently, Gerd Hoffmann, 2009/05/29
- Re: [Qemu-devel] [PATCH] use qemu_malloc and friends consistently, Paul Brook, 2009/05/29
- Re: [Qemu-devel] [PATCH] use qemu_malloc and friends consistently, Gerd Hoffmann, 2009/05/29
- Re: [Qemu-devel] [PATCH] use qemu_malloc and friends consistently, Glauber Costa, 2009/05/29
- Re: [Qemu-devel] [PATCH] use qemu_malloc and friends consistently, Anthony Liguori, 2009/05/29
- Re: [Qemu-devel] [PATCH] use qemu_malloc and friends consistently, malc, 2009/05/29
- Re: [Qemu-devel] [PATCH] use qemu_malloc and friends consistently, Julian Seward, 2009/05/29
- Re: [Qemu-devel] [PATCH] use qemu_malloc and friends consistently, Gerd Hoffmann, 2009/05/29
- Re: [Qemu-devel] [PATCH] use qemu_malloc and friends consistently, David Turner, 2009/05/29
- Re: [Qemu-devel] [PATCH] use qemu_malloc and friends consistently, David Turner, 2009/05/29
- Re: [Qemu-devel] [PATCH] use qemu_malloc and friends consistently,
Markus Armbruster <=
- Re: [Qemu-devel] [PATCH] use qemu_malloc and friends consistently, Gerd Hoffmann, 2009/05/29
- Re: [Qemu-devel] [PATCH] use qemu_malloc and friends consistently, malc, 2009/05/29
Re: [Qemu-devel] [PATCH] use qemu_malloc and friends consistently, jcd, 2009/05/29
Re: [Qemu-devel] [PATCH] use qemu_malloc and friends consistently, jcd, 2009/05/29
Re: [Qemu-devel] [PATCH] use qemu_malloc and friends consistently, jcd, 2009/05/29