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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 04/05/2010 01:51 PM, 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.

+#define VIR_ALLOC_VAR(ptr, type, count) virAllocVar(&(ptr), sizeof(*ptr), sizeof(type), (count))

Should that second argument be sizeof(*(ptr)) for safety?  On the other
hand, the parenthesis around count are redundant, if you want to strip them.

Right, thanks; I'll fix that.

Is everybody ok with having this allocator, btw?

Dave

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]