[libvirt] [PATCH 1/1] Implement variable length structure allocator

[Daniel P. Berrange]

On Mon, Apr 05, 2010 at 11:51:48AM -0600, Eric Blake wrote:
> On 04/05/2010 11:21 AM, David Allan wrote:
> > +int virAllocVar(void *ptrptr, size_t struct_size, size_t element_size, size_t count)
> 
> Shouldn't that be void **ptrptr - that is, the caller passes in the
> address of a void* that we then modify?
> 
> > +{
> > +    size_t alloc_size = 0;
> > +
> > +#if TEST_OOM
> > +    if (virAllocTestFail())
> > +        return -1;
> > +#endif
> > +
> > +    if (VIR_ALLOC_VAR_OVERSIZED(struct_size, count, element_size)) {
> > +        errno = ENOMEM;
> > +        return -1;
> > +    }
> > +
> > +    alloc_size = struct_size + (element_size * count);
> > +    *(void **)ptrptr = calloc(1, alloc_size);
> > +    if (*(void **)ptrptr == NULL)
> 
> Especially since typing it correctly to begin with would avoid these
> ugly type-punning casts?
> 
> > +++ b/src/util/memory.h
> > @@ -48,6 +48,10 @@
> >  int virAlloc(void *ptrptr, size_t size) ATTRIBUTE_RETURN_CHECK;
> >  int virAllocN(void *ptrptr, size_t size, size_t count) ATTRIBUTE_RETURN_CHECK;
> >  int virReallocN(void *ptrptr, size_t size, size_t count) ATTRIBUTE_RETURN_CHECK;
> > +int virAllocVar(void *ptrptr,
> > +                size_t struct_size,
> > +                size_t element_size,
> > +                size_t count) ATTRIBUTE_RETURN_CHECK;
> 
> Then again, fixing the type for your new method would imply fixing the
> typing of virAlloc and friends as well.

I did originally try void** in these methods, but it didn't work out
nicely, requiring extra casts in the macros. IIRC, the problem was that
if you had

  char *foo;

  VIR_ALLOC_N(foo)

then virAllocN would be given 'char **', which is compatible with void*
but is not compatible with void**, without doing an manual cast. So
switching these from void* to void** just moves where we need todo the
fugly casts & I preferred them hidden in the memory.c impl rather than
the header.

Daniel
-- 
|: Red Hat, Engineering, London    -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org        -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|