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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list