[libvirt] [PATCH 00/12] util: add VIR_(INSERT|DELETE)_ELEMENTS_N
PATCH 01/12 defines a couple new macros/functions to simplify inserting/deleting items in the middle of an array. The remaining patches switch over various pieces of existing code to use these new macros rather than directly calling memmove, etc. There are several other places where these macros could *almost* be used, except that they use the model of pre-allocating the larger array, then performing a bunch of operations (that may fail) and then finally move items around in the array and adjust its count. I'm wondering if I should add a flags arg to these to allow for specifying a "Don't realloc" flag - when it was given, the memory allocation operation would be presumed already done (in the case of insert) or to be done after return (for delete). Any opinions on that?
I noticed when writing the backend functions for virNetworkUpdate that I was repeating the same sequence of memmove, VIR_REALLOC, nXXX-- (and messed up the args to memmove at least once), and had seen the same sequence in a lot of other places, so I decided to write a couple utility functions/macros - see the .h file for full documentation. The intent is both to reduce the number of lines of code, but more importantly to eliminate the need to check the element size and element count arithmetic every time we need to do this (I *always* make at least one mistake.) VIR_INSERT_ELEMENTS_N: insert one or more elements at an arbitrary index within an array of objects. The size of each object is determined automatically by the macro using sizeof(*array). If a pointer to new elements is provided, they are copied in, otherwise the new space is set to all 0. VIR_APPEND_ELEMENTS_N: This is just a special case of VIR_INSERT_ELEMENTS_N that "inserts" one past the current last element. VIR_DELETE_ELEMENTS_N: delete one or more elements at an arbitrary index within an array of objects. It's assumed that the elements being deleted are already saved elsewhere (or cleared, if that's what is appropriate). Note that VIR_DELETE_ELEMENTS can return a failure, but only if an invalid index is given (index + amount to delete is > current array size), so in most cases you can safely ignore the return. --- src/libvirt_private.syms | 2 ++ src/util/memory.c | 77 +++++++++++++++++++++++++++++++++++++++++++++++- src/util/memory.h | 71 +++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 148 insertions(+), 2 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 60f9c7f..bead98c 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -821,8 +821,10 @@ virLogUnlock; virAlloc; virAllocN; virAllocVar; +virDeleteElementsN; virExpandN; virFree; +virInsertElementsN; virReallocN; virResizeN; virShrinkN; diff --git a/src/util/memory.c b/src/util/memory.c index 0f7aca1..46b6fdd 100644 --- a/src/util/memory.c +++ b/src/util/memory.c @@ -1,7 +1,7 @@ /* * memory.c: safer memory allocation * - * Copyright (C) 2010-2011 Red Hat, Inc. + * Copyright (C) 2010-2012 Red Hat, Inc. * Copyright (C) 2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -253,6 +253,81 @@ void virShrinkN(void *ptrptr, size_t size, size_t *countptr, size_t toremove) } } +/** + * virInsertElementsN: + * @ptrptr: pointer to hold address of allocated memory + * @countptr: variable tracking number of elements currently allocated + * @add: number of elements to add + * @at: index within array where new elements should be added + * @newelem: pointer to array of one or more new elements to move into place + * (the originals will be zeroed out if successful) + * + * Re-allocate an array of 'count' elements, each sizeof(*ptr) bytes + * long, to be 'count' + 'add' elements long, then appropriately move + * the elements starting at ptr[at] up by 'count' elements, copy the + * items from 'newelem' into ptr[at], then store the address of + * allocated memory in 'ptr' and the new size in 'count'. If + * 'newelem' is NULL, the new elements at ptr[at] are instead filled + * with zero. + * + * Returns -1 on failure, 0 on success + */ +int +virInsertElementsN(void *ptrptr, size_t size, size_t *countptr, + size_t add, size_t at, void *newelem) +{ + if ((at > *countptr) || (virExpandN(ptrptr, size, countptr, add) < 0)) + return -1; + + /* memory was successfully re-allocated. Move up all elements from + * ptrptr[at] to the end, memcpy in the new elements, and clear + * the elements from their original location. Remember that + * *countptr has already been updated with new element count! + */ + memmove(*(char**)ptrptr + (size * (at + add)), + *(char**)ptrptr + (size * at), + size * (*countptr - add - at)); + + if (newelem) { + memmove(*(char**)ptrptr + (size * at), newelem, size * add); + memset((char*)newelem, 0, size * add); + } else { + memset(*(char**)ptrptr + (size * at), 0, size * add); + } + return 0; +} + +/** + * virDeleteElementsN: + * @ptr: pointer to hold address of allocated memory + * @count: variable tracking number of elements currently allocated + * @remove: number of elements to remove + * @at: index within array where new elements should be deleted + * + * Re-allocate an array of 'count' elements, each sizeof(*ptr) + * bytes long, to be 'count' - 'remove' elements long, then store the + * address of allocated memory in 'ptr' and the new size in 'count'. + * If 'count' <= 'remove', the entire array is freed. + * + * Returns -1 on failure, 0 on success + */ +int +virDeleteElementsN(void *ptrptr, size_t size, size_t *countptr, + size_t remove, size_t at) +{ + if (at + remove > *countptr) + return -1; + + /* First move down the elements at the end that won't be deleted, + * then realloc. We assume that the items being deleted have + * already been cleared. + */ + memmove(*(char**)ptrptr + (size * at), + *(char**)ptrptr + (size * (at + remove)), + size * (*countptr - remove - at)); + virShrinkN(ptrptr, size, countptr, remove); + return 0; +} /** * Vir_Alloc_Var: diff --git a/src/util/memory.h b/src/util/memory.h index ad8ee64..5359ba0 100644 --- a/src/util/memory.h +++ b/src/util/memory.h @@ -1,7 +1,7 @@ /* * memory.c: safer memory allocation * - * Copyright (C) 2010-2011 Red Hat, Inc. + * Copyright (C) 2010-2012 Red Hat, Inc. * Copyright (C) 2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -59,6 +59,12 @@ int virResizeN(void *ptrptr, size_t size, size_t *alloc, size_t count, ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3); void virShrinkN(void *ptrptr, size_t size, size_t *count, size_t toremove) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3); +int virInsertElementsN(void *ptrptr, size_t size, size_t *countptr, + size_t add, size_t at, void *newelem) + ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3); +int virDeleteElementsN(void *ptrptr, size_t size, size_t *countptr, + size_t remove, size_t at) + ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3); int virAllocVar(void *ptrptr, size_t struct_size, size_t element_size, @@ -158,6 +164,69 @@ void virFree(void *ptrptr) ATTRIBUTE_NONNULL(1); # define VIR_SHRINK_N(ptr, count, remove) \ virShrinkN(&(ptr), sizeof(*(ptr)), &(count), remove) +/** + * VIR_INSERT_ELEMENTS_N: + * @ptr: pointer to hold address of allocated memory + * @count: variable tracking number of elements currently allocated + * @add: number of elements to add + * @at: index within array where new elements should be added + * @newelem: pointer to array of one or more new elements to move into place + * (the originals will be zeroed out if successful) + * + * Re-allocate an array of 'count' elements, each sizeof(*ptr) bytes + * long, to be 'count' + 'add' elements long, then appropriately move + * the elements starting at ptr[at] up by 'count' elements, copy the + * items from 'newelem' into ptr[at], then store the address of + * allocated memory in 'ptr' and the new size in 'count'. If + * 'newelem' is NULL, the new elements at ptr[at] are instead filled + * with zero. + * + * Returns -1 on failure, 0 on success + * + * + */ +# define VIR_INSERT_ELEMENTS_N(ptr, count, add, at, newelem) \ + virInsertElementsN(&(ptr), sizeof(*(ptr)), &(count), add, at, newelem) + +/** + * VIR_APPEND_ELEMENTS_N: + * @ptr: pointer to hold address of allocated memory + * @count: variable tracking number of elements currently allocated + * @add: number of elements to add + * @newelem: pointer to array of one or more new elements to move into place + * (the originals will be zeroed out if successful) + * + * Re-allocate an array of 'count' elements, each sizeof(*ptr) bytes + * long, to be 'count' + 'add' elements long, then copy the items from + * 'newelem' into ptr[count+1], then store the address of allocated + * memory in 'ptr' and the new size in 'count'. If 'newelem' is NULL, + * the new elements at ptr[at] are instead filled with zero. + * + * Returns -1 on failure, 0 on success + * + * + */ +# define VIR_APPEND_ELEMENTS_N(ptr, count, add, newelem) \ + virInsertElementsN(&(ptr), sizeof(*(ptr)), &(count), \ + add, (count) + 1, newelem) + +/** + * VIR_DELETE_ELEMENTS_N: + * @ptr: pointer to hold address of allocated memory + * @count: variable tracking number of elements currently allocated + * @remove: number of elements to remove + * @at: index within array where new elements should be deleted + * + * Re-allocate an array of 'count' elements, each sizeof(*ptr) + * bytes long, to be 'count' - 'remove' elements long, then store the + * address of allocated memory in 'ptr' and the new size in 'count'. + * If 'count' <= 'remove', the entire array is freed. + * + * No return value. + */ +# define VIR_DELETE_ELEMENTS_N(ptr, count, remove, at) \ + virDeleteElementsN(&(ptr), sizeof(*(ptr)), &(count), remove, at) + /* * VIR_ALLOC_VAR_OVERSIZED: * @M: size of base structure -- 1.7.11.7
The already-written backend functions for virNetworkUpdate that add and delete items into lists within the a network were already debugged to work properly, but future such functions will use VIR_(INSERT|DELETE)_ELEMENTS_N instead, so these are changed for uniformity. --- src/conf/network_conf.c | 94 +++++++++++++++---------------------------------- src/conf/network_conf.h | 10 +++--- 2 files changed, 33 insertions(+), 71 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 8ad72e7..a8c42a0 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -2453,27 +2453,15 @@ virNetworkDefUpdateIPDHCPHost(virNetworkDefPtr def, goto cleanup; } } + /* add to beginning/end of list */ - if (VIR_REALLOC_N(ipdef->hosts, ipdef->nhosts +1) < 0) { + if (VIR_INSERT_ELEMENTS_N(ipdef->hosts, + command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST + ? 0 : ipdef->nhosts, + ipdef->nhosts, 1, &host) < 0) { virReportOOMError(); goto cleanup; } - - if (command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST) { - - ipdef->hosts[ipdef->nhosts] = host; - ipdef->nhosts++; - memset(&host, 0, sizeof(host)); - - } else { /* implied (command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST) */ - - memmove(ipdef->hosts + 1, ipdef->hosts, - sizeof(*ipdef->hosts) * ipdef->nhosts); - ipdef->hosts[0] = host; - ipdef->nhosts++; - memset(&host, 0, sizeof(host)); - } - } else if (command == VIR_NETWORK_UPDATE_COMMAND_DELETE) { if (virNetworkDHCPHostDefParseXML(def->name, ctxt->node, &host, false) < 0) @@ -2499,10 +2487,8 @@ virNetworkDefUpdateIPDHCPHost(virNetworkDefPtr def, /* remove it */ virNetworkDHCPHostDefClear(&ipdef->hosts[ii]); - memmove(ipdef->hosts + ii, ipdef->hosts + ii + 1, - sizeof(*ipdef->hosts) * (ipdef->nhosts - ii - 1)); - ipdef->nhosts--; - ignore_value(VIR_REALLOC_N(ipdef->hosts, ipdef->nhosts)); + ignore_value(VIR_DELETE_ELEMENTS_N(ipdef->hosts, ii, ipdef->nhosts, 1)); + } else { virNetworkDefUpdateUnknownCommand(command); goto cleanup; @@ -2573,21 +2559,14 @@ virNetworkDefUpdateIPDHCPRange(virNetworkDefPtr def, } /* add to beginning/end of list */ - if (VIR_REALLOC_N(ipdef->ranges, ipdef->nranges +1) < 0) { + if (VIR_INSERT_ELEMENTS_N(ipdef->ranges, + command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST + ? 0 : ipdef->nranges, + ipdef->nranges, 1, &range) < 0) { virReportOOMError(); goto cleanup; } - if (command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST) { - ipdef->ranges[ipdef->nranges] = range; - } else { /* implied (command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST) */ - memmove(ipdef->ranges + 1, ipdef->ranges, - sizeof(*ipdef->ranges) * ipdef->nranges); - ipdef->ranges[0] = range; - } - ipdef->nranges++; - memset(&range, 0, sizeof(range)); - } else if (command == VIR_NETWORK_UPDATE_COMMAND_DELETE) { if (ii == ipdef->nranges) { @@ -2599,10 +2578,9 @@ virNetworkDefUpdateIPDHCPRange(virNetworkDefPtr def, /* remove it */ /* NB: nothing to clear from a RangeDef that's being freed */ - memmove(ipdef->ranges + ii, ipdef->ranges + ii + 1, - sizeof(*ipdef->ranges) * (ipdef->nranges - ii - 1)); - ipdef->nranges--; - ignore_value(VIR_REALLOC_N(ipdef->ranges, ipdef->nranges)); + ignore_value(VIR_DELETE_ELEMENTS_N(ipdef->ranges, ii, + ipdef->nranges, 1)); + } else { virNetworkDefUpdateUnknownCommand(command); goto cleanup; @@ -2677,21 +2655,14 @@ virNetworkDefUpdateForwardInterface(virNetworkDefPtr def, } /* add to beginning/end of list */ - if (VIR_REALLOC_N(def->forwardIfs, def->nForwardIfs + 1) < 0) { + if (VIR_INSERT_ELEMENTS_N(def->forwardIfs, + command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST + ? 0 : def->nForwardIfs, + def->nForwardIfs, 1, &iface) < 0) { virReportOOMError(); goto cleanup; } - if (command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST) { - def->forwardIfs[def->nForwardIfs] = iface; - } else { /* implied (command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST) */ - memmove(def->forwardIfs + 1, def->forwardIfs, - sizeof(*def->forwardIfs) * def->nForwardIfs); - def->forwardIfs[0] = iface; - } - def->nForwardIfs++; - memset(&iface, 0, sizeof(iface)); - } else if (command == VIR_NETWORK_UPDATE_COMMAND_DELETE) { if (ii == def->nForwardIfs) { @@ -2715,10 +2686,9 @@ virNetworkDefUpdateForwardInterface(virNetworkDefPtr def, /* remove it */ virNetworkForwardIfDefClear(&def->forwardIfs[ii]); - memmove(def->forwardIfs + ii, def->forwardIfs + ii + 1, - sizeof(*def->forwardIfs) * (def->nForwardIfs - ii - 1)); - def->nForwardIfs--; - ignore_value(VIR_REALLOC_N(def->forwardIfs, def->nForwardIfs)); + ignore_value(VIR_DELETE_ELEMENTS_N(def->forwardIfs, ii, + def->nForwardIfs, 1)); + } else { virNetworkDefUpdateUnknownCommand(command); goto cleanup; @@ -2813,29 +2783,21 @@ virNetworkDefUpdatePortGroup(virNetworkDefPtr def, (command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST)) { /* add to beginning/end of list */ - if (VIR_REALLOC_N(def->portGroups, def->nPortGroups +1) < 0) { + if (VIR_INSERT_ELEMENTS_N(def->portGroups, + command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST + ? 0 : def->nPortGroups, + def->nPortGroups, 1, &portgroup) < 0) { virReportOOMError(); goto cleanup; } - if (command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST) { - def->portGroups[def->nPortGroups] = portgroup; - } else { /* implied (command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST) */ - memmove(def->portGroups + 1, def->portGroups, - sizeof(*def->portGroups) * def->nPortGroups); - def->portGroups[0] = portgroup; - } - def->nPortGroups++; - memset(&portgroup, 0, sizeof(portgroup)); - } else if (command == VIR_NETWORK_UPDATE_COMMAND_DELETE) { /* remove it */ virPortGroupDefClear(&def->portGroups[foundName]); - memmove(def->portGroups + foundName, def->portGroups + foundName + 1, - sizeof(*def->portGroups) * (def->nPortGroups - foundName - 1)); - def->nPortGroups--; - ignore_value(VIR_REALLOC_N(def->portGroups, def->nPortGroups)); + ignore_value(VIR_DELETE_ELEMENTS_N(def->portGroups, foundName, + def->nPortGroups, 1)); + } else { virNetworkDefUpdateUnknownCommand(command); goto cleanup; diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index 1b22cc2..7d90a19 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -106,11 +106,11 @@ struct _virNetworkDNSHostDef { typedef struct _virNetworkDNSDef virNetworkDNSDef; typedef virNetworkDNSDef *virNetworkDNSDefPtr; struct _virNetworkDNSDef { - unsigned int ntxts; + size_t ntxts; virNetworkDNSTxtDefPtr txts; - unsigned int nhosts; + size_t nhosts; virNetworkDNSHostDefPtr hosts; - unsigned int nsrvs; + size_t nsrvs; virNetworkDNSSrvDefPtr srvs; }; @@ -131,10 +131,10 @@ struct _virNetworkIpDef { unsigned int prefix; /* ipv6 - only prefix allowed */ virSocketAddr netmask; /* ipv4 - either netmask or prefix specified */ - unsigned int nranges; /* Zero or more dhcp ranges */ + size_t nranges; /* Zero or more dhcp ranges */ virNetworkDHCPRangeDefPtr ranges; - unsigned int nhosts; /* Zero or more dhcp hosts */ + size_t nhosts; /* Zero or more dhcp hosts */ virNetworkDHCPHostDefPtr hosts; char *tftproot; -- 1.7.11.7
--- src/util/memory.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/util/memory.h b/src/util/memory.h index 5359ba0..872fec2 100644 --- a/src/util/memory.h +++ b/src/util/memory.h @@ -167,9 +167,9 @@ void virFree(void *ptrptr) ATTRIBUTE_NONNULL(1); /** * VIR_INSERT_ELEMENTS_N: * @ptr: pointer to hold address of allocated memory + * @at: index within array where new elements should be added * @count: variable tracking number of elements currently allocated * @add: number of elements to add - * @at: index within array where new elements should be added * @newelem: pointer to array of one or more new elements to move into place * (the originals will be zeroed out if successful) * @@ -185,7 +185,7 @@ void virFree(void *ptrptr) ATTRIBUTE_NONNULL(1); * * */ -# define VIR_INSERT_ELEMENTS_N(ptr, count, add, at, newelem) \ +# define VIR_INSERT_ELEMENTS_N(ptr, at, count, add, newelem) \ virInsertElementsN(&(ptr), sizeof(*(ptr)), &(count), add, at, newelem) /** @@ -213,9 +213,9 @@ void virFree(void *ptrptr) ATTRIBUTE_NONNULL(1); /** * VIR_DELETE_ELEMENTS_N: * @ptr: pointer to hold address of allocated memory + * @at: index within array where new elements should be deleted * @count: variable tracking number of elements currently allocated * @remove: number of elements to remove - * @at: index within array where new elements should be deleted * * Re-allocate an array of 'count' elements, each sizeof(*ptr) * bytes long, to be 'count' - 'remove' elements long, then store the @@ -224,7 +224,7 @@ void virFree(void *ptrptr) ATTRIBUTE_NONNULL(1); * * No return value. */ -# define VIR_DELETE_ELEMENTS_N(ptr, count, remove, at) \ +# define VIR_DELETE_ELEMENTS_N(ptr, at, count, remove) \ virDeleteElementsN(&(ptr), sizeof(*(ptr)), &(count), remove, at) /* -- 1.7.11.7
nvcpupin was declared as int, as were the nvcpupin args in several functions. These were all changed to size_t. virDomainVcpuPinAdd() was leaking a cpumask if VIR_REALLOC_N failed. This is completely inconsequential (since the only reason for failure was OOM, so we would already be dead at this time anyway), but I fixed the omission just to be tidy. --- src/conf/domain_conf.c | 47 ++++++++++++++--------------------------------- src/conf/domain_conf.h | 12 ++++++------ src/qemu/qemu_cgroup.c | 2 +- src/qemu/qemu_cgroup.h | 2 +- src/qemu/qemu_driver.c | 4 ++-- 5 files changed, 24 insertions(+), 43 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index acb6cb7..dda37fd 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1556,7 +1556,7 @@ virDomainClockDefClear(virDomainClockDefPtr def) } virDomainVcpuPinDefPtr * -virDomainVcpuPinDefCopy(virDomainVcpuPinDefPtr *src, int nvcpupin) +virDomainVcpuPinDefCopy(virDomainVcpuPinDefPtr *src, size_t nvcpupin) { int i = 0; virDomainVcpuPinDefPtr *ret = NULL; @@ -1601,9 +1601,9 @@ virDomainVcpuPinDefFree(virDomainVcpuPinDefPtr def) void virDomainVcpuPinDefArrayFree(virDomainVcpuPinDefPtr *def, - int nvcpupin) + size_t nvcpupin) { - int i; + size_t i; if (!def || !nvcpupin) return; @@ -11565,7 +11565,7 @@ int virDomainDefAddImplicitControllers(virDomainDefPtr def) * Return 1 if exists, 0 if not. */ int virDomainVcpuPinIsDuplicate(virDomainVcpuPinDefPtr *def, - int nvcpupin, + size_t nvcpupin, int vcpu) { int i; @@ -11583,7 +11583,7 @@ virDomainVcpuPinIsDuplicate(virDomainVcpuPinDefPtr *def, virDomainVcpuPinDefPtr virDomainVcpuPinFindByVcpu(virDomainVcpuPinDefPtr *def, - int nvcpupin, + size_t nvcpupin, int vcpu) { int i; @@ -11600,7 +11600,7 @@ virDomainVcpuPinFindByVcpu(virDomainVcpuPinDefPtr *def, } int virDomainVcpuPinAdd(virDomainVcpuPinDefPtr **vcpupin_list, - int *nvcpupin, + size_t *nvcpupin, unsigned char *cpumap, int maplen, int vcpu) @@ -11638,14 +11638,13 @@ int virDomainVcpuPinAdd(virDomainVcpuPinDefPtr **vcpupin_list, return -1; } - if (VIR_REALLOC_N(*vcpupin_list, *nvcpupin + 1) < 0) { + if (VIR_APPEND_ELEMENTS_N(*vcpupin_list, *nvcpupin, 1, &vcpupin) < 0) { virReportOOMError(); + VIR_FREE(vcpupin->cpumask); VIR_FREE(vcpupin); return -1; } - (*vcpupin_list)[(*nvcpupin)++] = vcpupin; - return 0; } @@ -11653,38 +11652,20 @@ int virDomainVcpuPinDel(virDomainDefPtr def, int vcpu) { int n; - bool deleted = false; - virDomainVcpuPinDefPtr *vcpupin_list = def->cputune.vcpupin; /* No vcpupin exists yet */ - if (!def->cputune.nvcpupin) { + if (!def->cputune.nvcpupin) return 0; - } for (n = 0; n < def->cputune.nvcpupin; n++) { - if (vcpupin_list[n]->vcpuid == vcpu) { - VIR_FREE(vcpupin_list[n]->cpumask); - VIR_FREE(vcpupin_list[n]); - memmove(&vcpupin_list[n], - &vcpupin_list[n+1], - (def->cputune.nvcpupin - n - 1) * sizeof(virDomainVcpuPinDef *)); - deleted = true; + if (def->cputune.vcpupin[n]->vcpuid == vcpu) { + VIR_FREE(def->cputune.vcpupin[n]->cpumask); + VIR_FREE(def->cputune.vcpupin[n]); + ignore_value(VIR_DELETE_ELEMENTS_N(def->cputune.vcpupin, n, + def->cputune.nvcpupin, 1)); break; } } - - if (!deleted) - return 0; - - if (--def->cputune.nvcpupin == 0) { - VIR_FREE(def->cputune.vcpupin); - } else { - if (VIR_REALLOC_N(def->cputune.vcpupin, def->cputune.nvcpupin) < 0) { - virReportOOMError(); - return -1; - } - } - return 0; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index c6f6e68..2f341be 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1624,17 +1624,17 @@ struct _virDomainVcpuPinDef { }; void virDomainVcpuPinDefFree(virDomainVcpuPinDefPtr def); -void virDomainVcpuPinDefArrayFree(virDomainVcpuPinDefPtr *def, int nvcpupin); +void virDomainVcpuPinDefArrayFree(virDomainVcpuPinDefPtr *def, size_t nvcpupin); virDomainVcpuPinDefPtr *virDomainVcpuPinDefCopy(virDomainVcpuPinDefPtr *src, - int nvcpupin); + size_t nvcpupin); int virDomainVcpuPinIsDuplicate(virDomainVcpuPinDefPtr *def, - int nvcpupin, + size_t nvcpupin, int vcpu); virDomainVcpuPinDefPtr virDomainVcpuPinFindByVcpu(virDomainVcpuPinDefPtr *def, - int nvcpupin, + size_t nvcpupin, int vcpu); typedef struct _virDomainNumatuneDef virDomainNumatuneDef; @@ -1704,7 +1704,7 @@ struct _virDomainDef { long long quota; unsigned long long emulator_period; long long emulator_quota; - int nvcpupin; + size_t nvcpupin; virDomainVcpuPinDefPtr *vcpupin; virDomainVcpuPinDefPtr emulatorpin; } cputune; @@ -1994,7 +1994,7 @@ int virDomainDefCompatibleDevice(virDomainDefPtr def, virDomainDeviceDefPtr dev); int virDomainVcpuPinAdd(virDomainVcpuPinDefPtr **vcpupin_list, - int *nvcpupin, + size_t *nvcpupin, unsigned char *cpumap, int maplen, int vcpu); diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index db371a0..48c68db 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -490,7 +490,7 @@ cleanup: int qemuSetupCgroupVcpuPin(virCgroupPtr cgroup, virDomainVcpuPinDefPtr *vcpupin, - int nvcpupin, + size_t nvcpupin, int vcpuid) { int i; diff --git a/src/qemu/qemu_cgroup.h b/src/qemu/qemu_cgroup.h index c552162..4819df5 100644 --- a/src/qemu/qemu_cgroup.h +++ b/src/qemu/qemu_cgroup.h @@ -53,7 +53,7 @@ int qemuSetupCgroupVcpuBW(virCgroupPtr cgroup, long long quota); int qemuSetupCgroupVcpuPin(virCgroupPtr cgroup, virDomainVcpuPinDefPtr *vcpupin, - int nvcpupin, + size_t nvcpupin, int vcpuid); int qemuSetupCgroupEmulatorPin(virCgroupPtr cgroup, virBitmapPtr cpumask); int qemuSetupCgroupForVcpu(struct qemud_driver *driver, virDomainObjPtr vm); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index feda4d9..c0d360a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3910,7 +3910,7 @@ qemudDomainPinVcpuFlags(virDomainPtr dom, int ret = -1; qemuDomainObjPrivatePtr priv; bool doReset = false; - int newVcpuPinNum = 0; + size_t newVcpuPinNum = 0; virDomainVcpuPinDefPtr *newVcpuPin = NULL; virBitmapPtr pcpumap = NULL; @@ -4184,7 +4184,7 @@ qemudDomainPinEmulator(virDomainPtr dom, int ret = -1; qemuDomainObjPrivatePtr priv; bool doReset = false; - int newVcpuPinNum = 0; + size_t newVcpuPinNum = 0; virDomainVcpuPinDefPtr *newVcpuPin = NULL; virBitmapPtr pcpumap = NULL; -- 1.7.11.7
--- src/conf/domain_conf.c | 68 +++++--------------------------------------------- 1 file changed, 6 insertions(+), 62 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index dda37fd..2a9dd80 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7830,19 +7830,7 @@ virDomainHostdevRemove(virDomainDefPtr def, size_t i) { virDomainHostdevDefPtr hostdev = def->hostdevs[i]; - if (def->nhostdevs > 1) { - memmove(def->hostdevs + i, - def->hostdevs + i + 1, - sizeof(*def->hostdevs) * - (def->nhostdevs - (i + 1))); - def->nhostdevs--; - if (VIR_REALLOC_N(def->hostdevs, def->nhostdevs) < 0) { - /* ignore, harmless */ - } - } else { - VIR_FREE(def->hostdevs); - def->nhostdevs = 0; - } + ignore_value(VIR_DELETE_ELEMENTS_N(def->hostdevs, i, def->nhostdevs, 1)); return hostdev; } @@ -8012,19 +8000,7 @@ virDomainDiskRemove(virDomainDefPtr def, size_t i) { virDomainDiskDefPtr disk = def->disks[i]; - if (def->ndisks > 1) { - memmove(def->disks + i, - def->disks + i + 1, - sizeof(*def->disks) * - (def->ndisks - (i + 1))); - def->ndisks--; - if (VIR_REALLOC_N(def->disks, def->ndisks) < 0) { - /* ignore, harmless */ - } - } else { - VIR_FREE(def->disks); - def->ndisks = 0; - } + ignore_value(VIR_DELETE_ELEMENTS_N(def->disks, i, def->ndisks, 1)); return disk; } @@ -8080,18 +8056,7 @@ virDomainNetRemove(virDomainDefPtr def, size_t i) } } } - if (def->nnets > 1) { - memmove(def->nets + i, - def->nets + i + 1, - sizeof(*def->nets) * (def->nnets - (i + 1))); - def->nnets--; - if (VIR_REALLOC_N(def->nets, def->nnets) < 0) { - /* ignore harmless */ - } - } else { - VIR_FREE(def->nets); - def->nnets = 0; - } + ignore_value(VIR_DELETE_ELEMENTS_N(def->nets, i, def->nnets, 1)); return net; } @@ -8179,20 +8144,8 @@ virDomainControllerRemove(virDomainDefPtr def, size_t i) { virDomainControllerDefPtr controller = def->controllers[i]; - if (def->ncontrollers > 1) { - memmove(def->controllers + i, - def->controllers + i + 1, - sizeof(*def->controllers) * - (def->ncontrollers - (i + 1))); - def->ncontrollers--; - if (VIR_REALLOC_N(def->controllers, def->ncontrollers) < 0) { - /* ignore, harmless */ - } - } else { - VIR_FREE(def->controllers); - def->ncontrollers = 0; - } - + ignore_value(VIR_DELETE_ELEMENTS_N(def->controllers, i, + def->ncontrollers, 1)); return controller; } @@ -8254,16 +8207,7 @@ virDomainLeaseRemoveAt(virDomainDefPtr def, size_t i) virDomainLeaseDefPtr lease = def->leases[i]; - if (def->nleases > 1) { - memmove(def->leases + i, - def->leases + i + 1, - sizeof(*def->leases) * - (def->nleases - (i + 1))); - VIR_SHRINK_N(def->leases, def->nleases, 1); - } else { - VIR_FREE(def->leases); - def->nleases = 0; - } + ignore_value(VIR_DELETE_ELEMENTS_N(def->leases, i, def->nleases, 1)); return lease; } -- 1.7.11.7
The stock remove functions in domain_conf.c already use VIR_DELETE_ELEMENTS_N. This does the same in a xen file. FIXME: This should be modified to call the helper functions in domain_conf.c. --- src/xen/xm_internal.c | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c index 7b3d340..7bc3ed7 100644 --- a/src/xen/xm_internal.c +++ b/src/xen/xm_internal.c @@ -1477,12 +1477,8 @@ xenXMDomainDetachDeviceFlags(virDomainPtr domain, const char *xml, dev->data.disk->dst && STREQ(def->disks[i]->dst, dev->data.disk->dst)) { virDomainDiskDefFree(def->disks[i]); - if (i < (def->ndisks - 1)) - memmove(def->disks + i, - def->disks + i + 1, - sizeof(*def->disks) * - (def->ndisks - (i + 1))); - def->ndisks--; + ignore_value(VIR_DELETE_ELEMENTS_N(def->disks, i, + def->ndisks, 1)); break; } } @@ -1494,12 +1490,8 @@ xenXMDomainDetachDeviceFlags(virDomainPtr domain, const char *xml, for (i = 0 ; i < def->nnets ; i++) { if (!virMacAddrCmp(&def->nets[i]->mac, &dev->data.net->mac)) { virDomainNetDefFree(def->nets[i]); - if (i < (def->nnets - 1)) - memmove(def->nets + i, - def->nets + i + 1, - sizeof(*def->nets) * - (def->nnets - (i + 1))); - def->nnets--; + ignore_value(VIR_DELETE_ELEMENTS_N(def->nets, i, + def->nnets, 1)); break; } } -- 1.7.11.7
Replaces memmove/VIR_REALLOC_N for xenUnifiedDomainList and XenStoreWatchList. --- src/xen/xen_driver.c | 14 +------------- src/xen/xen_driver.h | 4 ++-- src/xen/xen_inotify.c | 16 +++------------- src/xen/xs_internal.c | 14 ++------------ src/xen/xs_internal.h | 2 +- 5 files changed, 9 insertions(+), 41 deletions(-) diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index 6f08aad..969a3e1 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -2383,19 +2383,7 @@ xenUnifiedRemoveDomainInfo(xenUnifiedDomainInfoListPtr list, VIR_FREE(list->doms[i]->name); VIR_FREE(list->doms[i]); - - if (i < (list->count - 1)) - memmove(list->doms + i, - list->doms + i + 1, - sizeof(*(list->doms)) * - (list->count - (i + 1))); - - if (VIR_REALLOC_N(list->doms, - list->count - 1) < 0) { - ; /* Failure to reduce memory allocation isn't fatal */ - } - list->count--; - + ignore_value(VIR_DELETE_ELEMENTS_N(list->doms, i, list->count, 1)); return 0; } } diff --git a/src/xen/xen_driver.h b/src/xen/xen_driver.h index b3fbcff..e6ba867 100644 --- a/src/xen/xen_driver.h +++ b/src/xen/xen_driver.h @@ -1,7 +1,7 @@ /* * xen_unified.c: Unified Xen driver. * - * Copyright (C) 2007, 2010-2011 Red Hat, Inc. + * Copyright (C) 2007, 2010-2012 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -141,7 +141,7 @@ typedef struct _xenUnifiedDomainInfo xenUnifiedDomainInfo; typedef xenUnifiedDomainInfo *xenUnifiedDomainInfoPtr; struct _xenUnifiedDomainInfoList { - unsigned int count; + size_t count; xenUnifiedDomainInfoPtr *doms; }; typedef struct _xenUnifiedDomainInfoList xenUnifiedDomainInfoList; diff --git a/src/xen/xen_inotify.c b/src/xen/xen_inotify.c index 10b7834..eee22b4 100644 --- a/src/xen/xen_inotify.c +++ b/src/xen/xen_inotify.c @@ -4,7 +4,7 @@ * /etc/xen * /var/lib/xend/domains * - * Copyright (C) 2010-2011 Red Hat, Inc. + * Copyright (C) 2010-2012 Red Hat, Inc. * Copyright (C) 2008 VirtualIron * * This library is free software; you can redistribute it and/or @@ -174,18 +174,8 @@ xenInotifyXendDomainsDirRemoveEntry(virConnectPtr conn, if (!memcmp(uuid, priv->configInfoList->doms[i]->uuid, VIR_UUID_BUFLEN)) { VIR_FREE(priv->configInfoList->doms[i]->name); VIR_FREE(priv->configInfoList->doms[i]); - - if (i < (priv->configInfoList->count - 1)) - memmove(priv->configInfoList->doms + i, - priv->configInfoList->doms + i + 1, - sizeof(*(priv->configInfoList->doms)) * - (priv->configInfoList->count - (i + 1))); - - if (VIR_REALLOC_N(priv->configInfoList->doms, - priv->configInfoList->count - 1) < 0) { - ; /* Failure to reduce memory allocation isn't fatal */ - } - priv->configInfoList->count--; + ignore_value(VIR_DELETE_ELEMENTS_N(priv->configInfoList->doms, i, + priv->configInfoList->count, 1)); return 0; } } diff --git a/src/xen/xs_internal.c b/src/xen/xs_internal.c index a91d409..c18051e 100644 --- a/src/xen/xs_internal.c +++ b/src/xen/xs_internal.c @@ -1246,18 +1246,8 @@ int xenStoreRemoveWatch(virConnectPtr conn, VIR_FREE(list->watches[i]->path); VIR_FREE(list->watches[i]->token); VIR_FREE(list->watches[i]); - - if (i < (list->count - 1)) - memmove(list->watches + i, - list->watches + i + 1, - sizeof(*(list->watches)) * - (list->count - (i + 1))); - - if (VIR_REALLOC_N(list->watches, - list->count - 1) < 0) { - ; /* Failure to reduce memory allocation isn't fatal */ - } - list->count--; + ignore_value(VIR_DELETE_ELEMENTS_N(list->watches, i, + list->count, 1)); return 0; } } diff --git a/src/xen/xs_internal.h b/src/xen/xs_internal.h index 84d0d29..e3799b9 100644 --- a/src/xen/xs_internal.h +++ b/src/xen/xs_internal.h @@ -89,7 +89,7 @@ typedef struct _xenStoreWatch xenStoreWatch; typedef xenStoreWatch *xenStoreWatchPtr; struct _xenStoreWatchList { - unsigned int count; + size_t count; xenStoreWatchPtr *watches; }; typedef struct _xenStoreWatchList xenStoreWatchList; -- 1.7.11.7
The volume object list is also directly manipulated in the storage driver, test storage driver, and parallels storage driver (this should probably be consolidated). --- src/conf/interface_conf.c | 14 +++----------- src/conf/interface_conf.h | 4 ++-- src/conf/network_conf.c | 11 +---------- src/conf/network_conf.h | 2 +- src/conf/node_device_conf.c | 11 +---------- src/conf/node_device_conf.h | 4 ++-- src/conf/nwfilter_conf.c | 12 ++---------- src/conf/nwfilter_conf.h | 2 +- src/conf/storage_conf.c | 11 +---------- src/conf/storage_conf.h | 4 ++-- src/parallels/parallels_storage.c | 15 ++------------- src/storage/storage_backend_rbd.c | 2 +- src/storage/storage_driver.c | 13 ++----------- src/test/test_driver.c | 15 ++------------- 14 files changed, 23 insertions(+), 97 deletions(-) diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c index 31b9219..73a95e9 100644 --- a/src/conf/interface_conf.c +++ b/src/conf/interface_conf.c @@ -1,7 +1,7 @@ /* * interface_conf.c: interfaces XML handling * - * Copyright (C) 2006-2010 Red Hat, Inc. + * Copyright (C) 2006-2012 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -1313,16 +1313,8 @@ void virInterfaceRemove(virInterfaceObjListPtr interfaces, if (interfaces->objs[i] == iface) { virInterfaceObjUnlock(interfaces->objs[i]); virInterfaceObjFree(interfaces->objs[i]); - - if (i < (interfaces->count - 1)) - memmove(interfaces->objs + i, interfaces->objs + i + 1, - sizeof(*(interfaces->objs)) * (interfaces->count - (i + 1))); - - if (VIR_REALLOC_N(interfaces->objs, interfaces->count - 1) < 0) { - ; /* Failure to reduce memory allocation isn't fatal */ - } - interfaces->count--; - + ignore_value(VIR_DELETE_ELEMENTS_N(interfaces->objs, i, + interfaces->count, 1)); break; } virInterfaceObjUnlock(interfaces->objs[i]); diff --git a/src/conf/interface_conf.h b/src/conf/interface_conf.h index 1749629..bcfca62 100644 --- a/src/conf/interface_conf.h +++ b/src/conf/interface_conf.h @@ -1,7 +1,7 @@ /* * interface_conf.h: interface XML handling entry points * - * Copyright (C) 2006-2009 Red Hat, Inc. + * Copyright (C) 2006-2012 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -171,7 +171,7 @@ struct _virInterfaceObj { typedef struct _virInterfaceObjList virInterfaceObjList; typedef virInterfaceObjList *virInterfaceObjListPtr; struct _virInterfaceObjList { - unsigned int count; + size_t count; virInterfaceObjPtr *objs; }; diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index a8c42a0..b07b82d 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -494,16 +494,7 @@ void virNetworkRemoveInactive(virNetworkObjListPtr nets, if (nets->objs[i] == net) { virNetworkObjUnlock(nets->objs[i]); virNetworkObjFree(nets->objs[i]); - - if (i < (nets->count - 1)) - memmove(nets->objs + i, nets->objs + i + 1, - sizeof(*(nets->objs)) * (nets->count - (i + 1))); - - if (VIR_REALLOC_N(nets->objs, nets->count - 1) < 0) { - ; /* Failure to reduce memory allocation isn't fatal */ - } - nets->count--; - + ignore_value(VIR_DELETE_ELEMENTS_N(nets->objs, i, nets->count, 1)); break; } virNetworkObjUnlock(nets->objs[i]); diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index 7d90a19..01c975d 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -228,7 +228,7 @@ struct _virNetworkObj { typedef struct _virNetworkObjList virNetworkObjList; typedef virNetworkObjList *virNetworkObjListPtr; struct _virNetworkObjList { - unsigned int count; + size_t count; virNetworkObjPtr *objs; }; diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 41fa8e4..ab2b263 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -218,16 +218,7 @@ void virNodeDeviceObjRemove(virNodeDeviceObjListPtr devs, if (devs->objs[i] == dev) { virNodeDeviceObjUnlock(dev); virNodeDeviceObjFree(devs->objs[i]); - - if (i < (devs->count - 1)) - memmove(devs->objs + i, devs->objs + i + 1, - sizeof(*(devs->objs)) * (devs->count - (i + 1))); - - if (VIR_REALLOC_N(devs->objs, devs->count - 1) < 0) { - ; /* Failure to reduce memory allocation isn't fatal */ - } - devs->count--; - + ignore_value(VIR_DELETE_ELEMENTS_N(devs->objs, i, devs->count, 1)); break; } virNodeDeviceObjUnlock(dev); diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index e394042..ee02991 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -1,7 +1,7 @@ /* * node_device_conf.h: config handling for node devices * - * Copyright (C) 2010-2011 Red Hat, Inc. + * Copyright (C) 2010-2012 Red Hat, Inc. * Copyright (C) 2008 Virtual Iron Software, Inc. * Copyright (C) 2008 David F. Lively * @@ -199,7 +199,7 @@ struct _virNodeDeviceObj { typedef struct _virNodeDeviceObjList virNodeDeviceObjList; typedef virNodeDeviceObjList *virNodeDeviceObjListPtr; struct _virNodeDeviceObjList { - unsigned int count; + size_t count; virNodeDeviceObjPtr *objs; }; diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c index 27dbee8..3f7c49e 100644 --- a/src/conf/nwfilter_conf.c +++ b/src/conf/nwfilter_conf.c @@ -426,16 +426,8 @@ virNWFilterObjRemove(virNWFilterObjListPtr nwfilters, if (nwfilters->objs[i] == nwfilter) { virNWFilterObjUnlock(nwfilters->objs[i]); virNWFilterObjFree(nwfilters->objs[i]); - - if (i < (nwfilters->count - 1)) - memmove(nwfilters->objs + i, nwfilters->objs + i + 1, - sizeof(*(nwfilters->objs)) * (nwfilters->count - (i + 1))); - - if (VIR_REALLOC_N(nwfilters->objs, nwfilters->count - 1) < 0) { - ; /* Failure to reduce memory allocation isn't fatal */ - } - nwfilters->count--; - + ignore_value(VIR_DELETE_ELEMENTS_N(nwfilters->objs, i, + nwfilters->count, 1)); break; } virNWFilterObjUnlock(nwfilters->objs[i]); diff --git a/src/conf/nwfilter_conf.h b/src/conf/nwfilter_conf.h index 805fbe7..4a2a9f3 100644 --- a/src/conf/nwfilter_conf.h +++ b/src/conf/nwfilter_conf.h @@ -547,7 +547,7 @@ struct _virNWFilterObj { typedef struct _virNWFilterObjList virNWFilterObjList; typedef virNWFilterObjList *virNWFilterObjListPtr; struct _virNWFilterObjList { - unsigned int count; + size_t count; virNWFilterObjPtr *objs; }; diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 5d9e61a..4076720 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -402,16 +402,7 @@ virStoragePoolObjRemove(virStoragePoolObjListPtr pools, if (pools->objs[i] == pool) { virStoragePoolObjUnlock(pools->objs[i]); virStoragePoolObjFree(pools->objs[i]); - - if (i < (pools->count - 1)) - memmove(pools->objs + i, pools->objs + i + 1, - sizeof(*(pools->objs)) * (pools->count - (i + 1))); - - if (VIR_REALLOC_N(pools->objs, pools->count - 1) < 0) { - ; /* Failure to reduce memory allocation isn't fatal */ - } - pools->count--; - + ignore_value(VIR_DELETE_ELEMENTS_N(pools->objs, i, pools->count, 1)); break; } virStoragePoolObjUnlock(pools->objs[i]); diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index d509b13..24ece24 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -116,7 +116,7 @@ struct _virStorageVolDef { typedef struct _virStorageVolDefList virStorageVolDefList; typedef virStorageVolDefList *virStorageVolDefListPtr; struct _virStorageVolDefList { - unsigned int count; + size_t count; virStorageVolDefPtr *objs; }; @@ -318,7 +318,7 @@ struct _virStoragePoolObj { typedef struct _virStoragePoolObjList virStoragePoolObjList; typedef virStoragePoolObjList *virStoragePoolObjListPtr; struct _virStoragePoolObjList { - unsigned int count; + size_t count; virStoragePoolObjPtr *objs; }; diff --git a/src/parallels/parallels_storage.c b/src/parallels/parallels_storage.c index 112e288..c542345 100644 --- a/src/parallels/parallels_storage.c +++ b/src/parallels/parallels_storage.c @@ -1178,19 +1178,8 @@ parallelsStorageVolumeDelete(virStorageVolPtr vol, unsigned int flags) } virStorageVolDefFree(privvol); - - if (i < (privpool->volumes.count - 1)) - memmove(privpool->volumes.objs + i, - privpool->volumes.objs + i + 1, - sizeof(*(privpool->volumes.objs)) * - (privpool->volumes.count - (i + 1))); - - if (VIR_REALLOC_N(privpool->volumes.objs, - privpool->volumes.count - 1) < 0) { - ; /* Failure to reduce memory allocation isn't fatal */ - } - privpool->volumes.count--; - + ignore_value(VIR_DELETE_ELEMENTS_N(privpool->volumes.objs, i, + privpool->volumes.count, 1)); break; } } diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index 0c9bdcc..935cb74 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -345,7 +345,7 @@ static int virStorageBackendRBDRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, pool->volumes.objs[pool->volumes.count++] = vol; } - VIR_DEBUG("Found %d images in RBD pool %s", + VIR_DEBUG("Found %zu images in RBD pool %s", pool->volumes.count, pool->def->source.name); ret = 0; diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 28829d3..890a1f8 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -2193,17 +2193,8 @@ storageVolumeDelete(virStorageVolPtr obj, VIR_INFO("Deleting volume '%s' from storage pool '%s'", vol->name, pool->def->name); virStorageVolDefFree(vol); - vol = NULL; - - if (i < (pool->volumes.count - 1)) - memmove(pool->volumes.objs + i, pool->volumes.objs + i + 1, - sizeof(*(pool->volumes.objs)) * (pool->volumes.count - (i + 1))); - - if (VIR_REALLOC_N(pool->volumes.objs, pool->volumes.count - 1) < 0) { - ; /* Failure to reduce memory allocation isn't fatal */ - } - pool->volumes.count--; - + ignore_value(VIR_DELETE_ELEMENTS_N(pool->volumes.objs, i, + pool->volumes.count, 1)); break; } } diff --git a/src/test/test_driver.c b/src/test/test_driver.c index c9f9115..025e1e9 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -5024,19 +5024,8 @@ testStorageVolumeDelete(virStorageVolPtr vol, for (i = 0 ; i < privpool->volumes.count ; i++) { if (privpool->volumes.objs[i] == privvol) { virStorageVolDefFree(privvol); - - if (i < (privpool->volumes.count - 1)) - memmove(privpool->volumes.objs + i, - privpool->volumes.objs + i + 1, - sizeof(*(privpool->volumes.objs)) * - (privpool->volumes.count - (i + 1))); - - if (VIR_REALLOC_N(privpool->volumes.objs, - privpool->volumes.count - 1) < 0) { - ; /* Failure to reduce memory allocation isn't fatal */ - } - privpool->volumes.count--; - + ignore_value(VIR_DELETE_ELEMENTS_N(privpool->volumes.objs, i, + privpool->volumes.count, 1)); break; } } -- 1.7.11.7
In both pci and usb cases, the count that held the size of the list was int so it had to be changed to size_t. --- src/util/hostusb.c | 24 ++++++------------------ src/util/pci.c | 30 +++++++++--------------------- 2 files changed, 15 insertions(+), 39 deletions(-) diff --git a/src/util/hostusb.c b/src/util/hostusb.c index 81a9f5a..b0c87d6 100644 --- a/src/util/hostusb.c +++ b/src/util/hostusb.c @@ -56,7 +56,7 @@ struct _usbDevice { }; struct _usbDeviceList { - unsigned int count; + size_t count; usbDevice **devs; }; @@ -426,13 +426,11 @@ usbDeviceListAdd(usbDeviceList *list, return -1; } - if (VIR_REALLOC_N(list->devs, list->count+1) < 0) { + if (VIR_APPEND_ELEMENTS_N(list->devs, list->count, 1, &dev) < 0) { virReportOOMError(); return -1; } - list->devs[list->count++] = dev; - return 0; } @@ -461,22 +459,12 @@ usbDeviceListSteal(usbDeviceList *list, int i; for (i = 0; i < list->count; i++) { - if (list->devs[i]->bus != dev->bus || - list->devs[i]->dev != dev->dev) - continue; - + if (list->devs[i]->bus == dev->bus && + list->devs[i]->dev == dev->dev) { ret = list->devs[i]; - - if (i != list->count--) - memmove(&list->devs[i], - &list->devs[i+1], - sizeof(*list->devs) * (list->count - i)); - - if (VIR_REALLOC_N(list->devs, list->count) < 0) { - ; /* not fatal */ - } - + ignore_value(VIR_DELETE_ELEMENTS_N(list->devs, i, list->count, i)); break; + } } return ret; } diff --git a/src/util/pci.c b/src/util/pci.c index 83d86b7..7d90ef6 100644 --- a/src/util/pci.c +++ b/src/util/pci.c @@ -74,7 +74,7 @@ struct _pciDevice { }; struct _pciDeviceList { - unsigned count; + size_t count; pciDevice **devs; }; @@ -1525,13 +1525,11 @@ pciDeviceListAdd(pciDeviceList *list, return -1; } - if (VIR_REALLOC_N(list->devs, list->count+1) < 0) { + if (VIR_APPEND_ELEMENTS_N(list->devs, list->count, 1, &dev) < 0) { virReportOOMError(); return -1; } - list->devs[list->count++] = dev; - return 0; } @@ -1561,24 +1559,14 @@ pciDeviceListSteal(pciDeviceList *list, int i; for (i = 0; i < list->count; i++) { - if (list->devs[i]->domain != dev->domain || - list->devs[i]->bus != dev->bus || - list->devs[i]->slot != dev->slot || - list->devs[i]->function != dev->function) - continue; - - ret = list->devs[i]; - - if (i != --list->count) - memmove(&list->devs[i], - &list->devs[i+1], - sizeof(*list->devs) * (list->count-i)); - - if (VIR_REALLOC_N(list->devs, list->count) < 0) { - ; /* not fatal */ + if (list->devs[i]->domain == dev->domain && + list->devs[i]->bus == dev->bus && + list->devs[i]->slot == dev->slot && + list->devs[i]->function == dev->function) { + ret = list->devs[i]; + ignore_value(VIR_DELETE_ELEMENTS_N(list->devs, i, list->count, 1)); + break; } - - break; } return ret; } -- 1.7.11.7
--- src/util/threads-win32.c | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/src/util/threads-win32.c b/src/util/threads-win32.c index 142aa4f..cfeb1a0 100644 --- a/src/util/threads-win32.c +++ b/src/util/threads-win32.c @@ -1,7 +1,7 @@ /* * threads-win32.c: basic thread synchronization primitives * - * Copyright (C) 2009-2011 Red Hat, Inc. + * Copyright (C) 2009-2012 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -200,14 +200,7 @@ void virCondSignal(virCondPtr c) if (c->nwaiters) { HANDLE event = c->waiters[0]; - if (c->nwaiters > 1) - memmove(c->waiters, - c->waiters + 1, - sizeof(c->waiters[0]) * (c->nwaiters-1)); - if (VIR_REALLOC_N(c->waiters, c->nwaiters - 1) < 0) { - ; - } - c->nwaiters--; + ignore_value(VIR_DELETE_ELEMENTS_N(c->waiters, 0, c->nwaiters, 1)); SetEvent(event); } -- 1.7.11.7
--- src/rpc/virnetclient.c | 22 +++++----------------- 1 file changed, 5 insertions(+), 17 deletions(-) diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index 18f60c1..e6b651f 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -923,25 +923,13 @@ void virNetClientRemoveStream(virNetClientPtr client, virNetClientLock(client); size_t i; for (i = 0 ; i < client->nstreams ; i++) { - if (client->streams[i] == st) + if (client->streams[i] == st) { + virObjectUnref(st); + ignore_value(VIR_DELETE_ELEMENTS_N(client->streams, i, + client->nstreams, 1)); break; + } } - if (i == client->nstreams) - goto cleanup; - - if (client->nstreams > 1) { - memmove(client->streams + i, - client->streams + i + 1, - sizeof(*client->streams) * - (client->nstreams - (i + 1))); - VIR_SHRINK_N(client->streams, client->nstreams, 1); - } else { - VIR_FREE(client->streams); - client->nstreams = 0; - } - virObjectUnref(st); - -cleanup: virNetClientUnlock(client); } -- 1.7.11.7
--- tests/qemumonitortestutils.c | 20 ++------------------ 1 file changed, 2 insertions(+), 18 deletions(-) diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c index 7133c99..bc10270 100644 --- a/tests/qemumonitortestutils.c +++ b/tests/qemumonitortestutils.c @@ -133,15 +133,7 @@ static int qemuMonitorTestProcessCommandJSON(qemuMonitorTestPtr test, ret = qemuMonitorTestAddReponse(test, test->items[0]->response); qemuMonitorTestItemFree(test->items[0]); - if (test->nitems == 1) { - VIR_FREE(test->items); - test->nitems = 0; - } else { - memmove(test->items, - test->items + 1, - sizeof(test->items[0]) * (test->nitems - 1)); - VIR_SHRINK_N(test->items, test->nitems, 1); - } + ignore_value(VIR_DELETE_ELEMENTS_N(test->items, 0, test->nitems, 1)); } cleanup: @@ -176,15 +168,7 @@ static int qemuMonitorTestProcessCommandText(qemuMonitorTestPtr test, ret = qemuMonitorTestAddReponse(test, test->items[0]->response); qemuMonitorTestItemFree(test->items[0]); - if (test->nitems == 1) { - VIR_FREE(test->items); - test->nitems = 0; - } else { - memmove(test->items, - test->items + 1, - sizeof(test->items[0]) * (test->nitems - 1)); - VIR_SHRINK_N(test->items, test->nitems, 1); - } + ignore_value(VIR_DELETE_ELEMENTS_N(test->items, 0, test->nitems, 1)); } cleanup: -- 1.7.11.7
On 10/20/2012 05:20 AM, Laine Stump wrote:
PATCH 01/12 defines a couple new macros/functions to simplify inserting/deleting items in the middle of an array. The remaining patches switch over various pieces of existing code to use these new macros rather than directly calling memmove, etc.
I just noticed that I had forgotten to squash patch 03/12 into 01/12 before posting this, so I resent the series with that done. Please review that instead: https://www.redhat.com/archives/libvir-list/2012-October/msg01340.html
participants (1)
-
Laine Stump