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

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

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.

> 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.

> > +    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 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