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

[malc <av1474@comtv.ru>]

On Tue, 19 May 2009, Eduardo Habkost wrote:

> On Tue, May 19, 2009 at 06:48:25PM +0400, malc wrote:
> <snip>
> > > > > >> 
> > > > > >> Can't see what this hunk accomplishes.  If we remove it, the loop
> > > > > >> rejects, and we thus execute:
> > > > > >> 
> > > > 
> > > > Once again, on Linux/GLIBC it will, on AIX it wont.
> > > 
> > > Why not? It will. If nb_snapshots is 0, it won't enter the loop. The
> > > problem with that code was the "if (!s->snapshots)" check, not the
> > > qemu_mallocz(0) call.
> > 
> > Because qemu_mallocz on AIX will be terminated by oom_check.
> 
> That's exactly what the patch prevents from happening.

And i said as much:

<quote>
Again, it's pointless only with your proposed addition, otherwise
instead of 'could not open disk image' one would get an out of memory
error.
</quote>

> 
> 
> > 
> > > > 
> > > > And FWIW despite behaviour of malloc(0) being marked as implementation
> > > > defined i have sa far was unable to find any documentaiton (Linux man
> > > > pages, GLIBC info files) witht the actual definition, unlike on AIX
> > > > where man pages make it crystal clear what happens.
> > > 
> > > You don't need to have the exact behavior defined, as long as:
> > 
> > I certainly don't, the standard certainly says the implementation is
> > obliged to document it, that's what seprates implementation-defined
> > from unspecified behaviour.
> >  
> > > 1) You call free(p) later
> > > 2) You don't dereference the returned pointer (just like you can't
> > >    dereference p[n] on a malloc(n) block)
> > > 3) You don't assume anything about the returned value when size==0
> > > 
> > > My point is that this is valid malloc() usage, and there may be existing
> > > qemu code relying on that, and I don't see any reason to put a trap for
> > > code that would be valid malloc()/free() usage.
> > 
> > Okay, at this point we both expressed our points of view and have to
> > agree to disagree.
> 
> Agreed.
> 
> 
> <snip>
> > > >  
> > > > There is nothing implementation defined about realloc(whatever, 0), it
> > > > has a defined meaning in POSIX:
> > > > http://opengroup.org/onlinepubs/007908775/xsh/realloc.html
> > > > 
> > > > So it doesn't use 1.
> > > > 
> > > 
> > > realloc() return value is specified exactly the same way malloc() is:
> > > 
> > > "If size is 0, either a null pointer or a unique pointer that can be
> > > successfully passed to free() is returned."
> > 
> > Nope, quoting from above page:
> > 
> > If size is 0 and ptr is not a null pointer, the object pointed to is 
> > freed.
> 
> I quoted the above from exactly the same page.
> 
> I really hope you are not proposing to make qemu_realloc(p, 0) work but
> qemu_malloc(0) fail, because you would be breaking lots of
> realloc()/malloc() equivalency assumptions.

That's exactly what i'm proposing.

-- 
mailto:av1474@comtv.ru