Re: [Xen-devel] [PATCH 10/31] libxl: Crash (more sensibly) on malloc failure

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2012-04-11 at 11:24 +0100, Ian Jackson wrote:
> Ian Campbell writes ("Re: [Xen-devel] [PATCH 10/31] libxl: Crash (more 
> sensibly) on malloc failure"):
> > On Tue, 2012-04-10 at 20:07 +0100, Ian Jackson wrote:
> ...
> > > Consequently,
> > >  - New noreturn function libxl__alloc_failed which may be used for
> > >    printing a vaguely-useful error message, rather than simply
> > >    dereferencing a null pointer.
> > 
> > We got that in the next patch?
> 
> Uh ?  libxl__alloc_failed is in this patch.  I'm not sure what you
> mean.

Quoted the wrong bullet! From the wrong list!

I was trying to respond to:

> Things left to do:
>  - Provide versions of malloc, realloc and free which do the
>    checking but which do not enroll the pointer in the gc.

> > Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
> 
> Ta.
> 
> > > +    int new_maxsize = gc->alloc_maxsize * 2 + 25;
> > 
> > + 25? (I don't mind, seems even more arbitrary now that we have the *2
> > though).
> 
> Well, zero isn't adequate :-).  So yes, it's arbitrary.  25 is 100
> bytes (i386) or 200 bytes (amd64) which seems a reasonable initial
> overhead and will probably avoid triggering a realloc too often.

Why isn't just doubling each time adequate?

> > > +    assert(new_maxsize < INT_MAX / sizeof(void*) / 2);
> > > +    gc->alloc_ptrs = realloc(gc->alloc_ptrs, new_maxsize * sizeof(void 
> > > *));
> > > +    if (!gc->alloc_ptrs)
> > > +        libxl__alloc_failed(CTX, __func__, sizeof(void*), new_maxsize);
> > 
> > Strictly this should be "..., new_maxsize, sizeof(void*)" since the
> > arguments are nmemb and size?
> 
> I was going to say "we should do this the same way as fwrite and
> calloc" so I looked them up, and they have the nmemb and size
> arguments in THE OPPOSITE ORDER.  No wonder I can never remember!

I'd never noticed that, how confusing.

> I guess this is more like calloc and it should mirror libxl_calloc so
> the prototype is right and this call site is wrong.  Fixed.
> 
> Ian.



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel