[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] fix qemu_malloc() error check for size==0
From: |
malc |
Subject: |
Re: [Qemu-devel] [PATCH] fix qemu_malloc() error check for size==0 |
Date: |
Tue, 19 May 2009 18:55:11 +0400 (MSD) |
On Tue, 19 May 2009, Eduardo Habkost wrote:
> 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
>
Alas your list is not exhaustive:
3) Test the returned value against NULL
[Which is precisely what the qcow2 code did]
And
4) Accidentally derefence it
And here one would get away with it on certain subset of systems.
>
> > 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.
There is, due to the possibility of performing a 3) and a hard time
catching that (unless someone solves halting problem or subset applicable
to QEMU thereof)
--
mailto:address@hidden
- Re: [Qemu-devel] [PATCH] fix qemu_malloc() error check for size==0, (continued)
- Re: [Qemu-devel] [PATCH] fix qemu_malloc() error check for size==0, Eduardo Habkost, 2009/05/19
- Re: [Qemu-devel] [PATCH] fix qemu_malloc() error check for size==0, Jamie Lokier, 2009/05/19
- Re: [Qemu-devel] [PATCH] fix qemu_malloc() error check for size==0, Eduardo Habkost, 2009/05/19
- Re: [Qemu-devel] [PATCH] fix qemu_malloc() error check for size==0, Jamie Lokier, 2009/05/19
- Re: [Qemu-devel] [PATCH] fix qemu_malloc() error check for size==0, Eduardo Habkost, 2009/05/19
- Re: [Qemu-devel] [PATCH] fix qemu_malloc() error check for size==0, Jamie Lokier, 2009/05/19
- Re: [Qemu-devel] [PATCH] fix qemu_malloc() error check for size==0, Markus Armbruster, 2009/05/19
- Re: [Qemu-devel] [PATCH] fix qemu_malloc() error check for size==0, Eduardo Habkost, 2009/05/19
- Re: [Qemu-devel] [PATCH] fix qemu_malloc() error check for size==0, malc, 2009/05/19
- Re: [Qemu-devel] [PATCH] fix qemu_malloc() error check for size==0, Eduardo Habkost, 2009/05/19
- Re: [Qemu-devel] [PATCH] fix qemu_malloc() error check for size==0,
malc <=
- [PATCH] Make qemu_alloc()/qemu_realloc() return NULL for size==0 (was Re: [Qemu-devel] [PATCH] fix qemu_malloc() error check for size==0), Eduardo Habkost, 2009/05/19
- Re: [PATCH] Make qemu_alloc()/qemu_realloc() return NULL for size==0 (was Re: [Qemu-devel] [PATCH] fix qemu_malloc() error check for size==0), malc, 2009/05/19
- Re: [PATCH] Make qemu_alloc()/qemu_realloc() return NULL for size==0 (was Re: [Qemu-devel] [PATCH] fix qemu_malloc() error check for size==0), Eduardo Habkost, 2009/05/19
- Re: [PATCH] Make qemu_alloc()/qemu_realloc() return NULL for size==0 (was Re: [Qemu-devel] [PATCH] fix qemu_malloc() error check for size==0), Jamie Lokier, 2009/05/19
- Re: [PATCH] Make qemu_alloc()/qemu_realloc() return NULL for size==0 (was Re: [Qemu-devel] [PATCH] fix qemu_malloc() error check for size==0), Kevin Wolf, 2009/05/20
- [Qemu-devel] Re: [PATCH] Make qemu_alloc()/qemu_realloc() return NULL for size==0, Markus Armbruster, 2009/05/20
- [Qemu-devel] Re: [PATCH] Make qemu_alloc()/qemu_realloc() return NULL for size==0, malc, 2009/05/20
- Re: [Qemu-devel] [PATCH] fix qemu_malloc() error check for size==0, Jamie Lokier, 2009/05/19
- Re: [Qemu-devel] [PATCH] fix qemu_malloc() error check for size==0, Eduardo Habkost, 2009/05/19
- Re: [Qemu-devel] [PATCH] fix qemu_malloc() error check for size==0, malc, 2009/05/19