[libvirt] [PATCH 00/13] Utilize more of VIR_(APPEND|INSERT|DELETE)_ELEMENT
The aim of this patch is to substitute the following pattern: VIR_REALLOC(array, cnt + 1); array[cnt++] = new_elem; with shorter: VIR_APPEND_ELEMENT(array, cnt, new_elem); In addition, remove writable-only memmove() when removing elements, with VIR_DELETE_ELEMENT(). Completeness is not the goal. Yes, there can be a few untouched places left behind that could be transformed too. This is just a first shot to substitute obvious patterns. Michal Privoznik (13): conf: Utilize more of VIR_(APPEND|INSERT|DELETE)_ELEMENT src/lxc/: Utilize more of VIR_(APPEND|INSERT|DELETE)_ELEMENT src/nwfilter: Utilize more of VIR_(APPEND|INSERT|DELETE)_ELEMENT src/openvz: Utilize more of VIR_(APPEND|INSERT|DELETE)_ELEMENT src/parallels: Utilize more of VIR_(APPEND|INSERT|DELETE)_ELEMENT src/phyp: Utilize more of VIR_(APPEND|INSERT|DELETE)_ELEMENT src/qemu: Utilize more of VIR_(APPEND|INSERT|DELETE)_ELEMENT src/rpc: Utilize more of VIR_(APPEND|INSERT|DELETE)_ELEMENT src/storage: Utilize more of VIR_(APPEND|INSERT|DELETE)_ELEMENT src/test: Utilize more of VIR_(APPEND|INSERT|DELETE)_ELEMENT src/util: Utilize more of VIR_(APPEND|INSERT|DELETE)_ELEMENT src/xen: Utilize more of VIR_(APPEND|INSERT|DELETE)_ELEMENT src/xenxs: Utilize more of VIR_(APPEND|INSERT|DELETE)_ELEMENT src/conf/domain_conf.c | 120 +++++++-------------------------- src/conf/domain_conf.h | 4 +- src/conf/interface_conf.c | 26 +++---- src/conf/interface_conf.h | 2 +- src/conf/network_conf.c | 30 +++------ src/conf/network_conf.h | 4 +- src/conf/node_device_conf.c | 16 +---- src/conf/node_device_conf.h | 2 +- src/conf/nwfilter_conf.c | 33 +++------ src/conf/nwfilter_conf.h | 8 +-- src/conf/nwfilter_params.c | 12 +--- src/conf/nwfilter_params.h | 2 +- src/conf/object_event.c | 14 +--- src/conf/storage_conf.c | 16 +---- src/lxc/lxc_container.c | 9 +-- src/nwfilter/nwfilter_gentech_driver.c | 13 ++-- src/openvz/openvz_conf.c | 12 +--- src/parallels/parallels_driver.c | 15 ++--- src/parallels/parallels_storage.c | 32 +++------ src/phyp/phyp_driver.c | 16 ++--- src/phyp/phyp_driver.h | 2 +- src/qemu/qemu_command.c | 90 ++++++++++--------------- src/qemu/qemu_conf.c | 8 +-- src/qemu/qemu_conf.h | 2 +- src/qemu/qemu_driver.c | 14 ++-- src/qemu/qemu_monitor_json.c | 2 +- src/qemu/qemu_monitor_text.c | 3 +- src/rpc/virnetclient.c | 11 +-- src/rpc/virnetserver.c | 11 +-- src/storage/storage_backend_disk.c | 13 ++-- src/storage/storage_backend_fs.c | 5 +- src/storage/storage_backend_iscsi.c | 5 +- src/storage/storage_backend_logical.c | 8 +-- src/storage/storage_backend_mpath.c | 4 +- src/storage/storage_backend_rbd.c | 10 ++- src/storage/storage_backend_scsi.c | 4 +- src/test/test_driver.c | 41 +++-------- src/util/virjson.c | 12 +--- src/util/virlockspace.c | 12 +--- src/xen/xen_driver.c | 19 +----- src/xen/xen_driver.h | 2 +- src/xen/xen_inotify.c | 13 +--- src/xen/xm_internal.c | 18 +---- src/xen/xs_internal.c | 19 +----- src/xen/xs_internal.h | 2 +- src/xenxs/xen_sxpr.c | 43 +++++------- src/xenxs/xen_xm.c | 27 +++----- 47 files changed, 222 insertions(+), 564 deletions(-) -- 1.9.0
This fixes a possible double free. In virNetworkAssignDef() if virBitmapNew() fails, then virNetworkObjFree(network) is called. However, with network->def pointing to actual @def. So if caller frees @def again, ... Moreover, this fixes one possible memory leak too. In virInterfaceAssignDef() if appending to the list of interfaces fails, we ought to call virInterfaceObjFree() instead of bare VIR_FREE(). Although, in order to do that some array size variables needs to be turned into size_t rather than int. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/conf/domain_conf.c | 120 +++++++++----------------------------------- src/conf/domain_conf.h | 4 +- src/conf/interface_conf.c | 26 +++------- src/conf/interface_conf.h | 2 +- src/conf/network_conf.c | 30 ++++------- src/conf/network_conf.h | 4 +- src/conf/node_device_conf.c | 16 ++---- src/conf/node_device_conf.h | 2 +- src/conf/nwfilter_conf.c | 33 ++++-------- src/conf/nwfilter_conf.h | 8 +-- src/conf/nwfilter_params.c | 12 ++--- src/conf/nwfilter_params.h | 2 +- src/conf/object_event.c | 14 +----- src/conf/storage_conf.c | 16 ++---- src/qemu/qemu_driver.c | 4 +- 15 files changed, 76 insertions(+), 217 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1d5cc14..e1b0115 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10216,17 +10216,9 @@ void virDomainDiskInsertPreAlloced(virDomainDefPtr def, } } - /* No disks with this bus yet, so put at end of list */ - if (insertAt == -1) - insertAt = def->ndisks; - - if (insertAt < def->ndisks) - memmove(def->disks + insertAt + 1, - def->disks + insertAt, - (sizeof(def->disks[0]) * (def->ndisks-insertAt))); - - def->disks[insertAt] = disk; - def->ndisks++; + /* VIR_INSERT_ELEMENT_INPLACE will never return an error here. */ + ignore_value(VIR_INSERT_ELEMENT_INPLACE(def->disks, insertAt, + def->ndisks, disk)); } @@ -10235,19 +10227,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; - } + VIR_DELETE_ELEMENT(def->disks, i, def->ndisks); return disk; } @@ -10274,13 +10254,17 @@ virDomainHasDiskMirror(virDomainObjPtr vm) int virDomainNetInsert(virDomainDefPtr def, virDomainNetDefPtr net) { - if (VIR_REALLOC_N(def->nets, def->nnets + 1) < 0) + /* hostdev net devices must also exist in the hostdevs array */ + if (net->type == VIR_DOMAIN_NET_TYPE_HOSTDEV && + virDomainHostdevInsert(def, &net->data.hostdev.def) < 0) + return -1; + + if (VIR_APPEND_ELEMENT(def->nets, def->nnets, net) < 0) { + /* virDomainHostdevInsert just appends new hostdevs, so we are sure + * that the hostdev we've added a few lines above is at the end of + * array. Although, devices are indexed from zero ... */ + virDomainHostdevRemove(def, def->nhostdevs - 1); return -1; - def->nets[def->nnets] = net; - def->nnets++; - if (net->type == VIR_DOMAIN_NET_TYPE_HOSTDEV) { - /* hostdev net devices must also exist in the hostdevs array */ - return virDomainHostdevInsert(def, &net->data.hostdev.def); } return 0; } @@ -10360,19 +10344,7 @@ virDomainNetRemove(virDomainDefPtr def, size_t i) virDomainNetDefPtr net = def->nets[i]; virDomainNetRemoveHostdev(def, net); - - 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; - } + VIR_DELETE_ELEMENT(def->nets, i, def->nnets); return net; } @@ -10415,17 +10387,9 @@ void virDomainControllerInsertPreAlloced(virDomainDefPtr def, } } - /* No controllers with this bus yet, so put at end of list */ - if (insertAt == -1) - insertAt = def->ncontrollers; - - if (insertAt < def->ncontrollers) - memmove(def->controllers + insertAt + 1, - def->controllers + insertAt, - (sizeof(def->controllers[0]) * (def->ncontrollers-insertAt))); - - def->controllers[insertAt] = controller; - def->ncontrollers++; + /* VIR_INSERT_ELEMENT_INPLACE will never return an error here. */ + ignore_value(VIR_INSERT_ELEMENT_INPLACE(def->controllers, insertAt, + def->ncontrollers, controller)); } int @@ -10449,20 +10413,7 @@ 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; - } - + VIR_DELETE_ELEMENT(def->controllers, i, def->ncontrollers); return controller; } @@ -10520,16 +10471,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; - } + VIR_DELETE_ELEMENT(def->leases, i, def->nleases); return lease; } @@ -14451,7 +14393,7 @@ virDomainVcpuPinFindByVcpu(virDomainVcpuPinDefPtr *def, int virDomainVcpuPinAdd(virDomainVcpuPinDefPtr **vcpupin_list, - int *nvcpupin, + size_t *nvcpupin, unsigned char *cpumap, int maplen, int vcpu) @@ -14484,11 +14426,9 @@ virDomainVcpuPinAdd(virDomainVcpuPinDefPtr **vcpupin_list, if (!vcpupin->cpumask) goto error; - if (VIR_REALLOC_N(*vcpupin_list, *nvcpupin + 1) < 0) + if (VIR_APPEND_ELEMENT(*vcpupin_list, *nvcpupin, vcpupin) < 0) goto error; - (*vcpupin_list)[(*nvcpupin)++] = vcpupin; - return 0; error: @@ -14500,7 +14440,6 @@ int virDomainVcpuPinDel(virDomainDefPtr def, int vcpu) { int n; - bool deleted = false; virDomainVcpuPinDefPtr *vcpupin_list = def->cputune.vcpupin; /* No vcpupin exists yet */ @@ -14512,24 +14451,11 @@ virDomainVcpuPinDel(virDomainDefPtr def, int vcpu) if (vcpupin_list[n]->vcpuid == vcpu) { virBitmapFree(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; + VIR_DELETE_ELEMENT(vcpupin_list, n, def->cputune.nvcpupin); 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) - return -1; - } - return 0; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 2467f65..82d4c61 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2003,7 +2003,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; @@ -2410,7 +2410,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/conf/interface_conf.c b/src/conf/interface_conf.c index 8053307..09f7d92 100644 --- a/src/conf/interface_conf.c +++ b/src/conf/interface_conf.c @@ -1266,16 +1266,14 @@ virInterfaceObjPtr virInterfaceAssignDef(virInterfaceObjListPtr interfaces, return NULL; } virInterfaceObjLock(iface); + + if (VIR_APPEND_ELEMENT_COPY(interfaces->objs, + interfaces->count, iface) < 0) { + virInterfaceObjFree(iface); + return NULL; + } + iface->def = def; - - if (VIR_REALLOC_N(interfaces->objs, interfaces->count + 1) < 0) { - VIR_FREE(iface); - return NULL; - } - - interfaces->objs[interfaces->count] = iface; - interfaces->count++; - return iface; } @@ -1292,15 +1290,7 @@ void virInterfaceRemove(virInterfaceObjListPtr interfaces, 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--; - + VIR_DELETE_ELEMENT(interfaces->objs, i, interfaces->count); break; } virInterfaceObjUnlock(interfaces->objs[i]); diff --git a/src/conf/interface_conf.h b/src/conf/interface_conf.h index 0e22575..2c759bc 100644 --- a/src/conf/interface_conf.h +++ b/src/conf/interface_conf.h @@ -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 bac0465..5a2bd06 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -372,9 +372,6 @@ virNetworkAssignDef(virNetworkObjListPtr nets, return network; } - if (VIR_REALLOC_N(nets->objs, nets->count + 1) < 0) - return NULL; - if (VIR_ALLOC(network) < 0) return NULL; if (virMutexInit(&network->lock) < 0) { @@ -384,9 +381,9 @@ virNetworkAssignDef(virNetworkObjListPtr nets, return NULL; } virNetworkObjLock(network); - network->def = def; - if (!(network->class_id = virBitmapNew(CLASS_ID_BITMAP_SIZE))) + if (VIR_APPEND_ELEMENT_COPY(nets->objs, nets->count, network) < 0 || + !(network->class_id = virBitmapNew(CLASS_ID_BITMAP_SIZE))) goto error; /* The first three class IDs are already taken */ @@ -395,8 +392,6 @@ virNetworkAssignDef(virNetworkObjListPtr nets, ignore_value(virBitmapSetBit(network->class_id, 2)); network->def = def; - nets->objs[nets->count] = network; - nets->count++; return network; error: @@ -576,15 +571,7 @@ void virNetworkRemoveInactive(virNetworkObjListPtr nets, 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--; - + VIR_DELETE_ELEMENT(nets->objs, i, nets->count); break; } virNetworkObjUnlock(nets->objs[i]); @@ -900,14 +887,17 @@ virNetworkDNSHostDefParseXML(const char *networkName, if (cur->type == XML_ELEMENT_NODE && xmlStrEqual(cur->name, BAD_CAST "hostname")) { if (cur->children != NULL) { - if (VIR_REALLOC_N(def->names, def->nnames + 1) < 0) - goto error; - def->names[def->nnames++] = (char *)xmlNodeGetContent(cur); - if (!def->names[def->nnames - 1]) { + char *name = (char *) xmlNodeGetContent(cur); + + if (!name) { virReportError(VIR_ERR_XML_DETAIL, _("Missing hostname in network '%s' DNS HOST record"), networkName); } + if (VIR_APPEND_ELEMENT(def->names, def->nnames, name) < 0) { + VIR_FREE(name); + goto error; + } } } cur = cur->next; diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index 3abe180..1a864de 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -108,7 +108,7 @@ typedef struct _virNetworkDNSHostDef virNetworkDNSHostDef; typedef virNetworkDNSHostDef *virNetworkDNSHostDefPtr; struct _virNetworkDNSHostDef { virSocketAddr ip; - int nnames; + size_t nnames; char **names; }; @@ -294,7 +294,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 ea85cff..d86bbed 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -186,15 +186,13 @@ virNodeDeviceObjPtr virNodeDeviceAssignDef(virNodeDeviceObjListPtr devs, return NULL; } virNodeDeviceObjLock(device); - device->def = def; - if (VIR_REALLOC_N(devs->objs, devs->count+1) < 0) { - device->def = NULL; + if (VIR_APPEND_ELEMENT_COPY(devs->objs, devs->count, device) < 0){ virNodeDeviceObjUnlock(device); virNodeDeviceObjFree(device); return NULL; } - devs->objs[devs->count++] = device; + device->def = def; return device; @@ -213,15 +211,7 @@ void virNodeDeviceObjRemove(virNodeDeviceObjListPtr devs, 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--; - + VIR_DELETE_ELEMENT(devs->objs, i, devs->count); break; } virNodeDeviceObjUnlock(dev); diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index 9f3abe7..26116a1 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -204,7 +204,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 52e1c06..d25e0cc 100644 --- a/src/conf/nwfilter_conf.c +++ b/src/conf/nwfilter_conf.c @@ -399,15 +399,13 @@ virNWFilterRuleDefAddString(virNWFilterRuleDefPtr nwf, const char *string, size_t maxstrlen) { - if (VIR_REALLOC_N(nwf->strings, nwf->nstrings+1) < 0) - return NULL; + char *tmp; - if (VIR_STRNDUP(nwf->strings[nwf->nstrings], string, maxstrlen) < 0) - return NULL; + if (VIR_STRNDUP(tmp, string, maxstrlen) < 0 || + VIR_APPEND_ELEMENT_COPY(nwf->strings, nwf->nstrings, tmp) < 0) + VIR_FREE(tmp); - nwf->nstrings++; - - return nwf->strings[nwf->nstrings-1]; + return tmp; } @@ -425,15 +423,7 @@ virNWFilterObjRemove(virNWFilterObjListPtr nwfilters, 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--; - + VIR_DELETE_ELEMENT(nwfilters->objs, i, nwfilters->count); break; } virNWFilterObjUnlock(nwfilters->objs[i]); @@ -2593,11 +2583,11 @@ virNWFilterDefParseXML(xmlXPathContextPtr ctxt) { } if (entry->rule || entry->include) { - if (VIR_REALLOC_N(ret->filterEntries, ret->nentries+1) < 0) { + if (VIR_APPEND_ELEMENT_COPY(ret->filterEntries, + ret->nentries, entry) < 0) { virNWFilterEntryFree(entry); goto cleanup; } - ret->filterEntries[ret->nentries++] = entry; } else virNWFilterEntryFree(entry); } @@ -3029,15 +3019,14 @@ virNWFilterObjAssignDef(virNWFilterObjListPtr nwfilters, } virNWFilterObjLock(nwfilter); nwfilter->active = 0; - nwfilter->def = def; - if (VIR_REALLOC_N(nwfilters->objs, nwfilters->count + 1) < 0) { - nwfilter->def = NULL; + if (VIR_APPEND_ELEMENT_COPY(nwfilters->objs, + nwfilters->count, nwfilter) < 0) { virNWFilterObjUnlock(nwfilter); virNWFilterObjFree(nwfilter); return NULL; } - nwfilters->objs[nwfilters->count++] = nwfilter; + nwfilter->def = def; return nwfilter; } diff --git a/src/conf/nwfilter_conf.h b/src/conf/nwfilter_conf.h index 0d09b6a..8c59330 100644 --- a/src/conf/nwfilter_conf.h +++ b/src/conf/nwfilter_conf.h @@ -476,7 +476,7 @@ struct _virNWFilterRuleDef { size_t nVarAccess; virNWFilterVarAccessPtr *varAccess; - int nstrings; + size_t nstrings; char **strings; }; @@ -524,7 +524,7 @@ struct _virNWFilterDef { char *chainsuffix; virNWFilterChainPriority chainPriority; - int nentries; + size_t nentries; virNWFilterEntryPtr *filterEntries; }; @@ -547,7 +547,7 @@ struct _virNWFilterObj { typedef struct _virNWFilterObjList virNWFilterObjList; typedef virNWFilterObjList *virNWFilterObjListPtr; struct _virNWFilterObjList { - unsigned int count; + size_t count; virNWFilterObjPtr *objs; }; @@ -572,7 +572,7 @@ typedef virNWFilterTechDriver *virNWFilterTechDriverPtr; typedef struct _virNWFilterRuleInst virNWFilterRuleInst; typedef virNWFilterRuleInst *virNWFilterRuleInstPtr; struct _virNWFilterRuleInst { - int ndata; + size_t ndata; void **data; virNWFilterTechDriverPtr techdriver; }; diff --git a/src/conf/nwfilter_params.c b/src/conf/nwfilter_params.c index 589fcf8..2379aa7 100644 --- a/src/conf/nwfilter_params.c +++ b/src/conf/nwfilter_params.c @@ -256,13 +256,7 @@ virNWFilterVarValueDelNthValue(virNWFilterVarValuePtr val, unsigned int pos) case NWFILTER_VALUE_TYPE_ARRAY: if (pos < val->u.array.nValues) { VIR_FREE(val->u.array.values[pos]); - val->u.array.nValues--; - - if (pos < val->u.array.nValues) - memmove(&val->u.array.values[pos], - &val->u.array.values[pos + 1], - sizeof(val->u.array.values[0]) * - (val->u.array.nValues - pos)); + VIR_DELETE_ELEMENT(val->u.array.values, pos, val->u.array.nValues); return 0; } break; @@ -642,11 +636,11 @@ virNWFilterHashTablePut(virNWFilterHashTablePtr table, if (VIR_STRDUP(newName, name) < 0) return -1; - if (VIR_REALLOC_N(table->names, table->nNames + 1) < 0) { + if (VIR_APPEND_ELEMENT_COPY(table->names, + table->nNames, newName) < 0) { VIR_FREE(newName); return -1; } - table->names[table->nNames++] = newName; } if (virHashAddEntry(table->hashTable, name, val) < 0) { diff --git a/src/conf/nwfilter_params.h b/src/conf/nwfilter_params.h index 6b0b1a1..5e9777b 100644 --- a/src/conf/nwfilter_params.h +++ b/src/conf/nwfilter_params.h @@ -67,7 +67,7 @@ typedef virNWFilterHashTable *virNWFilterHashTablePtr; struct _virNWFilterHashTable { virHashTablePtr hashTable; - int nNames; + size_t nNames; char **names; }; diff --git a/src/conf/object_event.c b/src/conf/object_event.c index de45257..5ceca8a 100644 --- a/src/conf/object_event.c +++ b/src/conf/object_event.c @@ -266,8 +266,7 @@ virObjectEventCallbackListMarkDeleteID(virConnectPtr conn, static int virObjectEventCallbackListPurgeMarked(virObjectEventCallbackListPtr cbList) { - int old_count = cbList->count; - int n; + size_t n; for (n = 0; n < cbList->count; n++) { if (cbList->callbacks[n]->deleted) { virFreeCallback freecb = cbList->callbacks[n]->freecb; @@ -276,19 +275,10 @@ virObjectEventCallbackListPurgeMarked(virObjectEventCallbackListPtr cbList) virObjectUnref(cbList->callbacks[n]->conn); VIR_FREE(cbList->callbacks[n]); - if (n < (cbList->count - 1)) - memmove(cbList->callbacks + n, - cbList->callbacks + n + 1, - sizeof(*(cbList->callbacks)) * - (cbList->count - (n + 1))); - cbList->count--; + VIR_DELETE_ELEMENT(cbList->callbacks, n, cbList->count); n--; } } - if (cbList->count < old_count && - VIR_REALLOC_N(cbList->callbacks, cbList->count) < 0) { - ; /* Failure to reduce memory allocation isn't fatal */ - } return 0; } diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index e4232e9..ac323d0 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -458,15 +458,7 @@ virStoragePoolObjRemove(virStoragePoolObjListPtr pools, 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--; - + VIR_DELETE_ELEMENT(pools->objs, i, pools->count); break; } virStoragePoolObjUnlock(pools->objs[i]); @@ -1782,15 +1774,13 @@ virStoragePoolObjAssignDef(virStoragePoolObjListPtr pools, } virStoragePoolObjLock(pool); pool->active = 0; - pool->def = def; - if (VIR_REALLOC_N(pools->objs, pools->count+1) < 0) { - pool->def = NULL; + if (VIR_APPEND_ELEMENT_COPY(pools->objs, pools->count, pool) < 0) { virStoragePoolObjUnlock(pool); virStoragePoolObjFree(pool); return NULL; } - pools->objs[pools->count++] = pool; + pool->def = def; return pool; } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f3a1f58..72bb592 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4301,7 +4301,7 @@ qemuDomainPinVcpuFlags(virDomainPtr dom, int ret = -1; qemuDomainObjPrivatePtr priv; bool doReset = false; - int newVcpuPinNum = 0; + size_t newVcpuPinNum = 0; virDomainVcpuPinDefPtr *newVcpuPin = NULL; virBitmapPtr pcpumap = NULL; virQEMUDriverConfigPtr cfg = NULL; @@ -4575,7 +4575,7 @@ qemuDomainPinEmulator(virDomainPtr dom, int ret = -1; qemuDomainObjPrivatePtr priv; bool doReset = false; - int newVcpuPinNum = 0; + size_t newVcpuPinNum = 0; virDomainVcpuPinDefPtr *newVcpuPin = NULL; virBitmapPtr pcpumap = NULL; virQEMUDriverConfigPtr cfg = NULL; -- 1.9.0
On 03/07/2014 02:45 AM, Michal Privoznik wrote:
This fixes a possible double free. In virNetworkAssignDef() if virBitmapNew() fails, then virNetworkObjFree(network) is called. However, with network->def pointing to actual @def. So if caller frees @def again, ...
Moreover, this fixes one possible memory leak too. In virInterfaceAssignDef() if appending to the list of interfaces fails, we ought to call virInterfaceObjFree() instead of bare VIR_FREE().
Although, in order to do that some array size variables needs to be turned into size_t rather than int.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/conf/domain_conf.c | 120 +++++++++----------------------------------- src/conf/domain_conf.h | 4 +- src/conf/interface_conf.c | 26 +++------- src/conf/interface_conf.h | 2 +- src/conf/network_conf.c | 30 ++++------- src/conf/network_conf.h | 4 +- src/conf/node_device_conf.c | 16 ++---- src/conf/node_device_conf.h | 2 +- src/conf/nwfilter_conf.c | 33 ++++-------- src/conf/nwfilter_conf.h | 8 +-- src/conf/nwfilter_params.c | 12 ++--- src/conf/nwfilter_params.h | 2 +- src/conf/object_event.c | 14 +----- src/conf/storage_conf.c | 16 ++---- src/qemu/qemu_driver.c | 4 +- 15 files changed, 76 insertions(+), 217 deletions(-)
@@ -900,14 +887,17 @@ virNetworkDNSHostDefParseXML(const char *networkName, if (cur->type == XML_ELEMENT_NODE && xmlStrEqual(cur->name, BAD_CAST "hostname")) { if (cur->children != NULL) { - if (VIR_REALLOC_N(def->names, def->nnames + 1) < 0) - goto error; - def->names[def->nnames++] = (char *)xmlNodeGetContent(cur); - if (!def->names[def->nnames - 1]) { + char *name = (char *) xmlNodeGetContent(cur); + + if (!name) { virReportError(VIR_ERR_XML_DETAIL, _("Missing hostname in network '%s' DNS HOST record"), networkName); } + if (VIR_APPEND_ELEMENT(def->names, def->nnames, name) < 0) { + VIR_FREE(name); + goto error; + }
Pre-existing problem - if the xmlNodeGetContent failed, we reported the error but didn't 'goto error'. Your new code still has the problem; you need to add a 'goto error' in the 'if (!name)' block. Amazing how much more compact this is. ACK with the error fix. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/lxc/lxc_container.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index f2440ff..e034051 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -774,17 +774,18 @@ static int lxcContainerSetReadOnly(void) } while (getmntent_r(procmnt, &mntent, mntbuf, sizeof(mntbuf)) != NULL) { + char *tmp; if (STREQ(mntent.mnt_dir, "/") || STREQ(mntent.mnt_dir, "/.oldroot") || STRPREFIX(mntent.mnt_dir, "/.oldroot/") || lxcIsBasicMountLocation(mntent.mnt_dir)) continue; - if (VIR_REALLOC_N(mounts, nmounts + 1) < 0) + if (VIR_STRDUP(tmp, mntent.mnt_dir) < 0 || + VIR_APPEND_ELEMENT(mounts, nmounts, tmp) < 0) { + VIR_FREE(tmp); goto cleanup; - if (VIR_STRDUP(mounts[nmounts], mntent.mnt_dir) < 0) - goto cleanup; - nmounts++; + } } if (mounts) -- 1.9.0
On 03/07/2014 02:45 AM, Michal Privoznik wrote:
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/lxc/lxc_container.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
ACK -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/nwfilter/nwfilter_gentech_driver.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter_gentech_driver.c index 8c5cd57..ef3d4bd 100644 --- a/src/nwfilter/nwfilter_gentech_driver.c +++ b/src/nwfilter/nwfilter_gentech_driver.c @@ -110,10 +110,7 @@ int virNWFilterRuleInstAddData(virNWFilterRuleInstPtr res, void *data) { - if (VIR_REALLOC_N(res->data, res->ndata+1) < 0) - return -1; - res->data[res->ndata++] = data; - return 0; + return VIR_APPEND_ELEMENT(res->data, res->ndata, data); } @@ -381,7 +378,7 @@ _virNWFilterInstantiateRec(virNWFilterTechDriverPtr techdriver, virNWFilterDefPtr filter, const char *ifname, virNWFilterHashTablePtr vars, - int *nEntries, + size_t *nEntries, virNWFilterRuleInstPtr **insts, enum instCase useNewFilter, bool *foundNewFilter, virNWFilterDriverStatePtr driver) @@ -407,13 +404,11 @@ _virNWFilterInstantiateRec(virNWFilterTechDriverPtr techdriver, break; } - if (VIR_REALLOC_N(*insts, (*nEntries)+1) < 0) { + if (VIR_APPEND_ELEMENT_COPY(*insts, *nEntries, inst) < 0) { rc = -1; break; } - (*insts)[(*nEntries)++] = inst; - } else if (inc) { VIR_DEBUG("Instantiating filter %s", inc->filterref); obj = virNWFilterObjFindByName(&driver->nwfilters, inc->filterref); @@ -652,7 +647,7 @@ virNWFilterInstantiate(const unsigned char *vmuuid ATTRIBUTE_UNUSED, int rc; size_t j; int nptrs; - int nEntries = 0; + size_t nEntries = 0; virNWFilterRuleInstPtr *insts = NULL; void **ptrs = NULL; bool instantiate = true; -- 1.9.0
On 03/07/2014 02:45 AM, Michal Privoznik wrote:
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/nwfilter/nwfilter_gentech_driver.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-)
ACK -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/openvz/openvz_conf.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c index 0ea8243..11f048b 100644 --- a/src/openvz/openvz_conf.c +++ b/src/openvz/openvz_conf.c @@ -232,10 +232,8 @@ openvzReadNetworkConf(virDomainDefPtr def, if (VIR_STRDUP(net->data.ethernet.ipaddr, token) < 0) goto error; - if (VIR_REALLOC_N(def->nets, def->nnets + 1) < 0) + if (VIR_APPEND_ELEMENT_COPY(def->nets, def->nnets, net) < 0) goto error; - def->nets[def->nnets++] = net; - net = NULL; token = strtok_r(NULL, " ", &saveptr); } @@ -326,10 +324,8 @@ openvzReadNetworkConf(virDomainDefPtr def, p = ++next; } while (p < token + strlen(token)); - if (VIR_REALLOC_N(def->nets, def->nnets + 1) < 0) + if (VIR_APPEND_ELEMENT_COPY(def->nets, def->nnets, net) < 0) goto error; - def->nets[def->nnets++] = net; - net = NULL; token = strtok_r(NULL, ";", &saveptr); } @@ -450,10 +446,8 @@ openvzReadFSConf(virDomainDefPtr def, } } - if (VIR_REALLOC_N(def->fss, def->nfss + 1) < 0) + if (VIR_APPEND_ELEMENT(def->fss, def->nfss, fs) < 0) goto error; - def->fss[def->nfss++] = fs; - fs = NULL; VIR_FREE(temp); -- 1.9.0
On 03/07/2014 02:46 AM, Michal Privoznik wrote:
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/openvz/openvz_conf.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-)
ACK -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/parallels/parallels_driver.c | 15 ++++----------- src/parallels/parallels_storage.c | 32 ++++++++------------------------ 2 files changed, 12 insertions(+), 35 deletions(-) diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index 33260ef..2cba3ca 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -231,11 +231,9 @@ parallelsAddSerialInfo(virDomainChrDefPtr **serials, size_t *nserials, if (parallelsGetSerialInfo(chr, key, value)) goto cleanup; - if (VIR_REALLOC_N(*serials, *nserials + 1) < 0) + if (VIR_APPEND_ELEMENT(*serials, *nserials, chr) < 0) goto cleanup; - (*serials)[(*nserials)++] = chr; - return 0; cleanup: @@ -273,11 +271,9 @@ parallelsAddVideoInfo(virDomainDefPtr def, virJSONValuePtr value) if (VIR_ALLOC(accel) < 0) goto error; - if (VIR_REALLOC_N(def->videos, def->nvideos + 1) < 0) + if (VIR_APPEND_ELEMENT_COPY(def->videos, def->nvideos, video) < 0) goto error; - def->videos[def->nvideos++] = video; - video->type = VIR_DOMAIN_VIDEO_TYPE_VGA; video->vram = mem << 20; video->heads = 1; @@ -386,11 +382,9 @@ parallelsAddHddInfo(virDomainDefPtr def, const char *key, virJSONValuePtr value) if (parallelsGetHddInfo(def, disk, key, value)) goto error; - if (VIR_REALLOC_N(def->disks, def->ndisks + 1) < 0) + if (VIR_APPEND_ELEMENT(def->disks, def->ndisks, disk) < 0) goto error; - def->disks[def->ndisks++] = disk; - return 0; error: @@ -625,10 +619,9 @@ parallelsAddVNCInfo(virDomainDefPtr def, virJSONValuePtr jobj_root) gr->listens[0].type = VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS; - if (VIR_REALLOC_N(def->graphics, def->ngraphics + 1) < 0) + if (VIR_APPEND_ELEMENT(def->graphics, def->ngraphics, gr) < 0) goto cleanup; - def->graphics[def->ngraphics++] = gr; return 0; cleanup: diff --git a/src/parallels/parallels_storage.c b/src/parallels/parallels_storage.c index bb5066f..b5a1a4c 100644 --- a/src/parallels/parallels_storage.c +++ b/src/parallels/parallels_storage.c @@ -311,11 +311,9 @@ static int parallelsAddDiskVolume(virStoragePoolObjPtr pool, if (VIR_STRDUP(def->key, def->target.path) < 0) goto error; - if (VIR_REALLOC_N(pool->volumes.objs, pool->volumes.count + 1) < 0) + if (VIR_APPEND_ELEMENT(pool->volumes.objs, pool->volumes.count, def) < 0) goto error; - pool->volumes.objs[pool->volumes.count++] = def; - return 0; no_memory: virReportOOMError(); @@ -1229,9 +1227,6 @@ parallelsStorageVolDefineXML(virStoragePoolObjPtr pool, } } - if (VIR_REALLOC_N(pool->volumes.objs, pool->volumes.count + 1) < 0) - goto cleanup; - if (virAsprintf(&privvol->target.path, "%s/%s", pool->def->target.path, privvol->name) < 0) goto cleanup; @@ -1255,7 +1250,9 @@ parallelsStorageVolDefineXML(virStoragePoolObjPtr pool, pool->def->allocation); } - pool->volumes.objs[pool->volumes.count++] = privvol; + if (VIR_APPEND_ELEMENT_COPY(pool->volumes.objs, + pool->volumes.count, privvol) < 0) + goto cleanup; ret = privvol; privvol = NULL; @@ -1362,10 +1359,6 @@ parallelsStorageVolCreateXMLFrom(virStoragePoolPtr pool, privpool->def->available = (privpool->def->capacity - privpool->def->allocation); - if (VIR_REALLOC_N(privpool->volumes.objs, - privpool->volumes.count + 1) < 0) - goto cleanup; - if (virAsprintf(&privvol->target.path, "%s/%s", privpool->def->target.path, privvol->name) == -1) goto cleanup; @@ -1377,7 +1370,9 @@ parallelsStorageVolCreateXMLFrom(virStoragePoolPtr pool, privpool->def->available = (privpool->def->capacity - privpool->def->allocation); - privpool->volumes.objs[privpool->volumes.count++] = privvol; + if (VIR_APPEND_ELEMENT_COPY(privpool->volumes.objs, + privpool->volumes.count, privvol) < 0) + goto cleanup; ret = virGetStorageVol(pool->conn, privpool->def->name, privvol->name, privvol->key, @@ -1416,18 +1411,7 @@ int parallelsStorageVolDefRemove(virStoragePoolObjPtr privpool, 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--; - + VIR_DELETE_ELEMENT(privpool->volumes.objs, i, privpool->volumes.count); break; } } -- 1.9.0
On 03/07/2014 02:46 AM, Michal Privoznik wrote:
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/parallels/parallels_driver.c | 15 ++++----------- src/parallels/parallels_storage.c | 32 ++++++++------------------------ 2 files changed, 12 insertions(+), 35 deletions(-)
ACK -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/phyp/phyp_driver.c | 16 +++++++--------- src/phyp/phyp_driver.h | 2 +- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 9adb6b0..e819256 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -570,19 +570,16 @@ phypUUIDTable_AddLpar(virConnectPtr conn, unsigned char *uuid, int id) { phyp_driverPtr phyp_driver = conn->privateData; uuid_tablePtr uuid_table = phyp_driver->uuid_table; + lparPtr item = NULL; - uuid_table->nlpars++; - size_t i = uuid_table->nlpars; - i--; - - if (VIR_REALLOC_N(uuid_table->lpars, uuid_table->nlpars) < 0) + if (VIR_ALLOC(item) < 0) goto err; - if (VIR_ALLOC(uuid_table->lpars[i]) < 0) - goto err; + item->id = id; + memcpy(item->uuid, uuid, VIR_UUID_BUFLEN); - uuid_table->lpars[i]->id = id; - memcpy(uuid_table->lpars[i]->uuid, uuid, VIR_UUID_BUFLEN); + if (VIR_APPEND_ELEMENT_COPY(uuid_table->lpars, uuid_table->nlpars, item) < 0) + goto err; if (phypUUIDTable_WriteFile(conn) == -1) goto err; @@ -593,6 +590,7 @@ phypUUIDTable_AddLpar(virConnectPtr conn, unsigned char *uuid, int id) return 0; err: + VIR_FREE(item); return -1; } diff --git a/src/phyp/phyp_driver.h b/src/phyp/phyp_driver.h index ad05b15..a5e7369 100644 --- a/src/phyp/phyp_driver.h +++ b/src/phyp/phyp_driver.h @@ -56,7 +56,7 @@ struct _lpar { typedef struct _uuid_table uuid_table_t; typedef uuid_table_t *uuid_tablePtr; struct _uuid_table { - int nlpars; + size_t nlpars; lparPtr *lpars; }; -- 1.9.0
On 03/07/2014 02:46 AM, Michal Privoznik wrote:
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/phyp/phyp_driver.c | 16 +++++++--------- src/phyp/phyp_driver.h | 2 +- 2 files changed, 8 insertions(+), 10 deletions(-)
ACK -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_command.c | 90 ++++++++++++++++++-------------------------- src/qemu/qemu_conf.c | 8 +--- src/qemu/qemu_conf.h | 2 +- src/qemu/qemu_driver.c | 10 ++--- src/qemu/qemu_monitor_json.c | 2 +- src/qemu/qemu_monitor_text.c | 3 +- 6 files changed, 46 insertions(+), 69 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 611d21d..648cf29 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -10981,15 +10981,16 @@ qemuParseCommandLineCPU(virDomainDefPtr dom, } if (j == dom->clock.ntimers) { - if (VIR_REALLOC_N(dom->clock.timers, j + 1) < 0 || - VIR_ALLOC(dom->clock.timers[j]) < 0) + virDomainTimerDefPtr timer; + if (VIR_ALLOC(timer) < 0 || + VIR_APPEND_ELEMENT_COPY(dom->clock.timers, + dom->clock.ntimers, timer) < 0) goto cleanup; - dom->clock.timers[j]->name = VIR_DOMAIN_TIMER_NAME_KVMCLOCK; - dom->clock.timers[j]->present = present; - dom->clock.timers[j]->tickpolicy = -1; - dom->clock.timers[j]->track = -1; - dom->clock.timers[j]->mode = -1; - dom->clock.ntimers++; + timer->name = VIR_DOMAIN_TIMER_NAME_KVMCLOCK; + timer->present = present; + timer->tickpolicy = -1; + timer->track = -1; + timer->mode = -1; } else if (dom->clock.timers[j]->present != -1 && dom->clock.timers[j]->present != present) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", @@ -11225,7 +11226,7 @@ qemuParseCommandLine(virCapsPtr qemuCaps, bool nographics = false; bool fullscreen = false; char *path; - int nnics = 0; + size_t nnics = 0; const char **nics = NULL; int video = VIR_DOMAIN_VIDEO_TYPE_CIRRUS; int nvirtiodisk = 0; @@ -11318,11 +11319,9 @@ qemuParseCommandLine(virCapsPtr qemuCaps, if (STREQ(arg, "-net")) { WANT_VALUE(); - if (STRPREFIX(val, "nic")) { - if (VIR_REALLOC_N(nics, nnics+1) < 0) - goto error; - nics[nnics++] = val; - } + if (STRPREFIX(val, "nic") && + VIR_APPEND_ELEMENT(nics, nnics, val) < 0) + goto error; } } @@ -11456,11 +11455,10 @@ qemuParseCommandLine(virCapsPtr qemuCaps, vnc->data.vnc.autoport = false; } - if (VIR_REALLOC_N(def->graphics, def->ngraphics+1) < 0) { + if (VIR_APPEND_ELEMENT(def->graphics, def->ngraphics, vnc) < 0) { virDomainGraphicsDefFree(vnc); goto error; } - def->graphics[def->ngraphics++] = vnc; } else if (STREQ(arg, "-m")) { int mem; WANT_VALUE(); @@ -11592,10 +11590,8 @@ qemuParseCommandLine(virCapsPtr qemuCaps, goto error; } - if (VIR_REALLOC_N(def->disks, def->ndisks+1) < 0) + if (VIR_APPEND_ELEMENT(def->disks, def->ndisks, disk) < 0) goto error; - def->disks[def->ndisks++] = disk; - disk = NULL; } else if (STREQ(arg, "-no-acpi")) { def->features[VIR_DOMAIN_FEATURE_ACPI] = VIR_DOMAIN_FEATURE_STATE_DEFAULT; } else if (STREQ(arg, "-no-reboot")) { @@ -11732,13 +11728,12 @@ qemuParseCommandLine(virCapsPtr qemuCaps, virDomainChrDefFree(chr); goto error; } - if (VIR_REALLOC_N(def->serials, def->nserials+1) < 0) { - virDomainChrDefFree(chr); - goto error; - } chr->deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL; chr->target.port = def->nserials; - def->serials[def->nserials++] = chr; + if (VIR_APPEND_ELEMENT(def->serials, def->nserials, chr) < 0) { + virDomainChrDefFree(chr); + goto error; + } } } else if (STREQ(arg, "-parallel")) { WANT_VALUE(); @@ -11752,13 +11747,12 @@ qemuParseCommandLine(virCapsPtr qemuCaps, virDomainChrDefFree(chr); goto error; } - if (VIR_REALLOC_N(def->parallels, def->nparallels+1) < 0) { - virDomainChrDefFree(chr); - goto error; - } chr->deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_PARALLEL; chr->target.port = def->nparallels; - def->parallels[def->nparallels++] = chr; + if (VIR_APPEND_ELEMENT(def->parallels, def->nparallels, chr) < 0) { + virDomainChrDefFree(chr); + goto error; + } } } else if (STREQ(arg, "-usbdevice")) { WANT_VALUE(); @@ -11794,19 +11788,16 @@ qemuParseCommandLine(virCapsPtr qemuCaps, disk->removable = VIR_DOMAIN_FEATURE_STATE_DEFAULT; if (VIR_STRDUP(disk->dst, "sda") < 0) goto error; - if (VIR_REALLOC_N(def->disks, def->ndisks+1) < 0) + if (VIR_APPEND_ELEMENT(def->disks, def->ndisks, disk) < 0) goto error; - def->disks[def->ndisks++] = disk; - disk = NULL; } else { virDomainHostdevDefPtr hostdev; if (!(hostdev = qemuParseCommandLineUSB(val))) goto error; - if (VIR_REALLOC_N(def->hostdevs, def->nhostdevs+1) < 0) { + if (VIR_APPEND_ELEMENT(def->hostdevs, def->nhostdevs, hostdev) < 0) { virDomainHostdevDefFree(hostdev); goto error; } - def->hostdevs[def->nhostdevs++] = hostdev; } } else if (STREQ(arg, "-net")) { WANT_VALUE(); @@ -11814,11 +11805,10 @@ qemuParseCommandLine(virCapsPtr qemuCaps, virDomainNetDefPtr net; if (!(net = qemuParseCommandLineNet(xmlopt, val, nnics, nics))) goto error; - if (VIR_REALLOC_N(def->nets, def->nnets+1) < 0) { + if (VIR_APPEND_ELEMENT(def->nets, def->nnets, net) < 0) { virDomainNetDefFree(net); goto error; } - def->nets[def->nnets++] = net; } } else if (STREQ(arg, "-drive")) { WANT_VALUE(); @@ -11826,23 +11816,19 @@ qemuParseCommandLine(virCapsPtr qemuCaps, nvirtiodisk, ceph_args != NULL))) goto error; - if (VIR_REALLOC_N(def->disks, def->ndisks+1) < 0) - goto error; if (disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO) nvirtiodisk++; - - def->disks[def->ndisks++] = disk; - disk = NULL; + if (VIR_APPEND_ELEMENT(def->disks, def->ndisks, disk) < 0) + goto error; } else if (STREQ(arg, "-pcidevice")) { virDomainHostdevDefPtr hostdev; WANT_VALUE(); if (!(hostdev = qemuParseCommandLinePCI(val))) goto error; - if (VIR_REALLOC_N(def->hostdevs, def->nhostdevs+1) < 0) { + if (VIR_APPEND_ELEMENT(def->hostdevs, def->nhostdevs, hostdev) < 0) { virDomainHostdevDefFree(hostdev); goto error; } - def->hostdevs[def->nhostdevs++] = hostdev; } else if (STREQ(arg, "-soundhw")) { const char *start; WANT_VALUE(); @@ -11867,11 +11853,10 @@ qemuParseCommandLine(virCapsPtr qemuCaps, if (VIR_ALLOC(snd) < 0) goto error; snd->model = type; - if (VIR_REALLOC_N(def->sounds, def->nsounds+1) < 0) { + if (VIR_APPEND_ELEMENT(def->sounds, def->nsounds, snd) < 0) { VIR_FREE(snd); goto error; } - def->sounds[def->nsounds++] = snd; } start = tmp ? tmp + 1 : NULL; @@ -12009,16 +11994,17 @@ qemuParseCommandLine(virCapsPtr qemuCaps, } else if (STREQ(arg, "-S")) { /* ignore, always added by libvirt */ } else { + char *tmp = NULL; /* something we can't yet parse. Add it to the qemu namespace * cmdline/environment advanced options and hope for the best */ VIR_WARN("unknown QEMU argument '%s', adding to the qemu namespace", arg); - if (VIR_REALLOC_N(cmd->args, cmd->num_args+1) < 0) + if (VIR_STRDUP(tmp, arg) < 0 || + VIR_APPEND_ELEMENT(cmd->args, cmd->num_args, tmp) < 0) { + VIR_FREE(tmp); goto error; - if (VIR_STRDUP(cmd->args[cmd->num_args], arg) < 0) - goto error; - cmd->num_args++; + } } } @@ -12112,11 +12098,10 @@ qemuParseCommandLine(virCapsPtr qemuCaps, goto error; } - if (VIR_REALLOC_N(def->graphics, def->ngraphics+1) < 0) { + if (VIR_APPEND_ELEMENT(def->graphics, def->ngraphics, sdl) < 0) { virDomainGraphicsDefFree(sdl); goto error; } - def->graphics[def->ngraphics++] = sdl; } if (def->ngraphics) { @@ -12132,11 +12117,10 @@ qemuParseCommandLine(virCapsPtr qemuCaps, virDomainVideoDefaultRAM(def, vid->type) : 0; vid->heads = 1; - if (VIR_REALLOC_N(def->videos, def->nvideos+1) < 0) { + if (VIR_APPEND_ELEMENT(def->videos, def->nvideos, vid) < 0) { virDomainVideoDefFree(vid); goto error; } - def->videos[def->nvideos++] = vid; } /* diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 2c397b0..20fd62d 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1084,13 +1084,7 @@ qemuRemoveSharedDevice(virQEMUDriverPtr driver, if (!(new_entry = qemuSharedDeviceEntryCopy(entry))) goto cleanup; - if (idx != new_entry->ref - 1) - memmove(&new_entry->domains[idx], - &new_entry->domains[idx + 1], - sizeof(*new_entry->domains) * (new_entry->ref - idx - 1)); - - VIR_SHRINK_N(new_entry->domains, new_entry->ref, 1); - + VIR_DELETE_ELEMENT(new_entry->domains, idx, new_entry->ref); if (virHashUpdateEntry(driver->sharedDevices, key, new_entry) < 0){ qemuSharedDeviceEntryFree(new_entry, NULL); goto cleanup; diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index ece185b..358a449 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -248,7 +248,7 @@ struct _virQEMUDriver { typedef struct _qemuDomainCmdlineDef qemuDomainCmdlineDef; typedef qemuDomainCmdlineDef *qemuDomainCmdlineDefPtr; struct _qemuDomainCmdlineDef { - unsigned int num_args; + size_t num_args; char **args; unsigned int num_env; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 72bb592..0f267c6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4044,17 +4044,17 @@ static int qemuDomainHotplugVcpus(virQEMUDriverPtr driver, */ virDomainVcpuPinDefPtr vcpupin = NULL; - if (VIR_REALLOC_N(vm->def->cputune.vcpupin, - vm->def->cputune.nvcpupin + 1) < 0) - goto cleanup; - if (VIR_ALLOC(vcpupin) < 0) goto cleanup; vcpupin->cpumask = virBitmapNew(VIR_DOMAIN_CPUMASK_LEN); virBitmapCopy(vcpupin->cpumask, vm->def->cpumask); vcpupin->vcpuid = i; - vm->def->cputune.vcpupin[vm->def->cputune.nvcpupin++] = vcpupin; + if (VIR_APPEND_ELEMENT_COPY(vm->def->cputune.vcpupin, + vm->def->cputune.nvcpupin, vcpupin) < 0) { + virBitmapFree(vcpupin->cpumask); + VIR_FREE(vcpupin); + } if (cgroup_vcpu) { if (qemuSetupCgroupVcpuPin(cgroup_vcpu, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index ee3ae15..7d6b88b 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1212,7 +1212,7 @@ qemuMonitorJSONExtractCPUInfo(virJSONValuePtr reply, goto cleanup; } - if (VIR_REALLOC_N(threads, ncpus) < 0) + if (VIR_ALLOC_N(threads, ncpus) < 0) goto cleanup; for (i = 0; i < ncpus; i++) { diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 7b81079..f4992f1 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -529,11 +529,10 @@ int qemuMonitorTextGetCPUInfo(qemuMonitorPtr mon, if (end == NULL || !c_isspace(*end)) goto error; - if (VIR_REALLOC_N(cpupids, ncpupids+1) < 0) + if (VIR_APPEND_ELEMENT_COPY(cpupids, ncpupids, tid) < 0) goto error; VIR_DEBUG("tid=%d", tid); - cpupids[ncpupids++] = tid; /* Skip to next data line */ line = strchr(offset, '\r'); -- 1.9.0
On 03/07/2014 02:46 AM, Michal Privoznik wrote:
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_command.c | 90 ++++++++++++++++++-------------------------- src/qemu/qemu_conf.c | 8 +--- src/qemu/qemu_conf.h | 2 +- src/qemu/qemu_driver.c | 10 ++--- src/qemu/qemu_monitor_json.c | 2 +- src/qemu/qemu_monitor_text.c | 3 +- 6 files changed, 46 insertions(+), 69 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 611d21d..648cf29 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -10981,15 +10981,16 @@ qemuParseCommandLineCPU(virDomainDefPtr dom, }
if (j == dom->clock.ntimers) { - if (VIR_REALLOC_N(dom->clock.timers, j + 1) < 0 || - VIR_ALLOC(dom->clock.timers[j]) < 0) + virDomainTimerDefPtr timer; + if (VIR_ALLOC(timer) < 0 || + VIR_APPEND_ELEMENT_COPY(dom->clock.timers, + dom->clock.ntimers, timer) < 0) goto cleanup;
Memory leak if timer is allocated but the append fails.
+++ b/src/qemu/qemu_driver.c @@ -4044,17 +4044,17 @@ static int qemuDomainHotplugVcpus(virQEMUDriverPtr driver, */ virDomainVcpuPinDefPtr vcpupin = NULL;
- if (VIR_REALLOC_N(vm->def->cputune.vcpupin, - vm->def->cputune.nvcpupin + 1) < 0) - goto cleanup; - if (VIR_ALLOC(vcpupin) < 0) goto cleanup;
vcpupin->cpumask = virBitmapNew(VIR_DOMAIN_CPUMASK_LEN); virBitmapCopy(vcpupin->cpumask, vm->def->cpumask); vcpupin->vcpuid = i; - vm->def->cputune.vcpupin[vm->def->cputune.nvcpupin++] = vcpupin; + if (VIR_APPEND_ELEMENT_COPY(vm->def->cputune.vcpupin, + vm->def->cputune.nvcpupin, vcpupin) < 0) { + virBitmapFree(vcpupin->cpumask); + VIR_FREE(vcpupin); + }
Missing a 'goto error;'. ACK with those fixed. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/rpc/virnetclient.c | 11 +---------- src/rpc/virnetserver.c | 11 ++--------- 2 files changed, 3 insertions(+), 19 deletions(-) diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index 9deec9e..327768b 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -920,16 +920,7 @@ void virNetClientRemoveStream(virNetClientPtr client, 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; - } + VIR_DELETE_ELEMENT(client->streams, i, client->nstreams); virObjectUnref(st); cleanup: diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index f70e260..bcabfcd 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -1130,15 +1130,8 @@ void virNetServerRun(virNetServerPtr srv) virNetServerClientClose(srv->clients[i]); if (virNetServerClientIsClosed(srv->clients[i])) { virNetServerClientPtr client = srv->clients[i]; - if (srv->nclients > 1) { - memmove(srv->clients + i, - srv->clients + i + 1, - sizeof(*srv->clients) * (srv->nclients - (i + 1))); - VIR_SHRINK_N(srv->clients, srv->nclients, 1); - } else { - VIR_FREE(srv->clients); - srv->nclients = 0; - } + + VIR_DELETE_ELEMENT(srv->clients, i, srv->nclients); /* Enable services if we can accept a new client. * The new client can be accepted if we are at the limit. */ -- 1.9.0
On 03/07/2014 02:46 AM, Michal Privoznik wrote:
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/rpc/virnetclient.c | 11 +---------- src/rpc/virnetserver.c | 11 ++--------- 2 files changed, 3 insertions(+), 19 deletions(-)
ACK -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/storage/storage_backend_disk.c | 13 ++++--------- src/storage/storage_backend_fs.c | 5 +---- src/storage/storage_backend_iscsi.c | 5 +---- src/storage/storage_backend_logical.c | 8 +++----- src/storage/storage_backend_mpath.c | 4 +--- src/storage/storage_backend_rbd.c | 10 ++++------ src/storage/storage_backend_scsi.c | 4 +--- 7 files changed, 15 insertions(+), 34 deletions(-) diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c index a7a7d0e..b261773 100644 --- a/src/storage/storage_backend_disk.c +++ b/src/storage/storage_backend_disk.c @@ -52,20 +52,15 @@ virStorageBackendDiskMakeDataVol(virStoragePoolObjPtr pool, if (vol == NULL) { if (VIR_ALLOC(vol) < 0) return -1; - - if (VIR_REALLOC_N(pool->volumes.objs, - pool->volumes.count+1) < 0) { - virStorageVolDefFree(vol); - return -1; - } - pool->volumes.objs[pool->volumes.count++] = vol; - /* Prepended path will be same for all partitions, so we can * strip the path to form a reasonable pool-unique name */ tmp = strrchr(groups[0], '/'); - if (VIR_STRDUP(vol->name, tmp ? tmp + 1 : groups[0]) < 0) + if (VIR_STRDUP(vol->name, tmp ? tmp + 1 : groups[0]) < 0 || + VIR_APPEND_ELEMENT(pool->volumes.objs, pool->volumes.count, vol) < 0) { + virStorageVolDefFree(vol); return -1; + } } if (vol->target.path == NULL) { diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 4d69f74..bab02dd 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -905,11 +905,8 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn ATTRIBUTE_UNUSED, } - if (VIR_REALLOC_N(pool->volumes.objs, - pool->volumes.count+1) < 0) + if (VIR_APPEND_ELEMENT(pool->volumes.objs, pool->volumes.count, vol) < 0) goto cleanup; - pool->volumes.objs[pool->volumes.count++] = vol; - vol = NULL; } closedir(dir); diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c index 3d75215..ada6c48 100644 --- a/src/storage/storage_backend_iscsi.c +++ b/src/storage/storage_backend_iscsi.c @@ -447,14 +447,11 @@ virStorageBackendISCSIGetTargets(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, if (VIR_STRDUP(target, groups[1]) < 0) return -1; - if (VIR_REALLOC_N(list->targets, list->ntargets + 1) < 0) { + if (VIR_APPEND_ELEMENT(list->targets, list->ntargets, target) < 0) { VIR_FREE(target); return -1; } - list->targets[list->ntargets] = target; - list->ntargets++; - return 0; } diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 15b86dc..1ac48e6 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -115,9 +115,6 @@ virStorageBackendLogicalMakeVol(virStoragePoolObjPtr pool, if (VIR_STRDUP(vol->name, groups[0]) < 0) goto cleanup; - if (VIR_REALLOC_N(pool->volumes.objs, - pool->volumes.count + 1)) - goto cleanup; } if (vol->target.path == NULL) { @@ -251,8 +248,9 @@ virStorageBackendLogicalMakeVol(virStoragePoolObjPtr pool, vol->source.nextent++; } - if (is_new_vol) - pool->volumes.objs[pool->volumes.count++] = vol; + if (is_new_vol && + VIR_APPEND_ELEMENT(pool->volumes.objs, pool->volumes.count, vol) < 0) + goto cleanup; ret = 0; diff --git a/src/storage/storage_backend_mpath.c b/src/storage/storage_backend_mpath.c index 1e65a8d..5d0ed32 100644 --- a/src/storage/storage_backend_mpath.c +++ b/src/storage/storage_backend_mpath.c @@ -99,10 +99,8 @@ virStorageBackendMpathNewVol(virStoragePoolObjPtr pool, if (VIR_STRDUP(vol->key, vol->target.path) < 0) goto cleanup; - if (VIR_REALLOC_N(pool->volumes.objs, - pool->volumes.count + 1) < 0) + if (VIR_APPEND_ELEMENT_COPY(pool->volumes.objs, pool->volumes.count, vol) < 0) goto cleanup; - pool->volumes.objs[pool->volumes.count++] = vol; pool->def->capacity += vol->capacity; pool->def->allocation += vol->allocation; ret = 0; diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index 22ed7de..bc52474 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -382,11 +382,6 @@ static int virStorageBackendRBDRefreshPool(virConnectPtr conn, for (name = names; name < names + max_size;) { virStorageVolDefPtr vol; - if (VIR_REALLOC_N(pool->volumes.objs, pool->volumes.count + 1) < 0) { - virStoragePoolObjClearVols(pool); - goto cleanup; - } - if (STREQ(name, "")) break; @@ -405,7 +400,10 @@ static int virStorageBackendRBDRefreshPool(virConnectPtr conn, goto cleanup; } - pool->volumes.objs[pool->volumes.count++] = vol; + if (VIR_APPEND_ELEMENT(pool->volumes.objs, pool->volumes.count, vol) < 0) { + virStoragePoolObjClearVols(pool); + goto cleanup; + } } VIR_DEBUG("Found %zu images in RBD pool %s", diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c index 1841816..4397257 100644 --- a/src/storage/storage_backend_scsi.c +++ b/src/storage/storage_backend_scsi.c @@ -249,12 +249,10 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr pool, pool->def->capacity += vol->capacity; pool->def->allocation += vol->allocation; - if (VIR_REALLOC_N(pool->volumes.objs, - pool->volumes.count + 1) < 0) { + if (VIR_APPEND_ELEMENT(pool->volumes.objs, pool->volumes.count, vol) < 0) { retval = -1; goto free_vol; } - pool->volumes.objs[pool->volumes.count++] = vol; goto out; -- 1.9.0
On 03/07/2014 02:46 AM, Michal Privoznik wrote:
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/storage/storage_backend_disk.c | 13 ++++--------- src/storage/storage_backend_fs.c | 5 +---- src/storage/storage_backend_iscsi.c | 5 +---- src/storage/storage_backend_logical.c | 8 +++----- src/storage/storage_backend_mpath.c | 4 +--- src/storage/storage_backend_rbd.c | 10 ++++------ src/storage/storage_backend_scsi.c | 4 +--- 7 files changed, 15 insertions(+), 34 deletions(-)
ACK -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/test/test_driver.c | 41 +++++++++-------------------------------- 1 file changed, 9 insertions(+), 32 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 6806ffd..986f215 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -1243,10 +1243,6 @@ testOpenVolumesForPool(const char *file, if (!def) goto error; - if (VIR_REALLOC_N(pool->volumes.objs, - pool->volumes.count+1) < 0) - goto error; - if (def->target.path == NULL) { if (virAsprintf(&def->target.path, "%s/%s", pool->def->target.path, @@ -1256,12 +1252,12 @@ testOpenVolumesForPool(const char *file, if (!def->key && VIR_STRDUP(def->key, def->target.path) < 0) goto error; + if (VIR_APPEND_ELEMENT_COPY(pool->volumes.objs, pool->volumes.count, def) < 0) + goto error; pool->def->allocation += def->allocation; pool->def->available = (pool->def->capacity - pool->def->allocation); - - pool->volumes.objs[pool->volumes.count++] = def; def = NULL; } @@ -5459,24 +5455,20 @@ testStorageVolCreateXML(virStoragePoolPtr pool, goto cleanup; } - if (VIR_REALLOC_N(privpool->volumes.objs, - privpool->volumes.count+1) < 0) - goto cleanup; - if (virAsprintf(&privvol->target.path, "%s/%s", privpool->def->target.path, privvol->name) == -1) goto cleanup; - if (VIR_STRDUP(privvol->key, privvol->target.path) < 0) + if (VIR_STRDUP(privvol->key, privvol->target.path) < 0 || + VIR_APPEND_ELEMENT_COPY(privpool->volumes.objs, + privpool->volumes.count, privvol) < 0) goto cleanup; privpool->def->allocation += privvol->allocation; privpool->def->available = (privpool->def->capacity - privpool->def->allocation); - privpool->volumes.objs[privpool->volumes.count++] = privvol; - ret = virGetStorageVol(pool->conn, privpool->def->name, privvol->name, privvol->key, NULL, NULL); @@ -5547,24 +5539,20 @@ testStorageVolCreateXMLFrom(virStoragePoolPtr pool, privpool->def->available = (privpool->def->capacity - privpool->def->allocation); - if (VIR_REALLOC_N(privpool->volumes.objs, - privpool->volumes.count+1) < 0) - goto cleanup; - if (virAsprintf(&privvol->target.path, "%s/%s", privpool->def->target.path, privvol->name) == -1) goto cleanup; - if (VIR_STRDUP(privvol->key, privvol->target.path) < 0) + if (VIR_STRDUP(privvol->key, privvol->target.path) < 0 || + VIR_APPEND_ELEMENT_COPY(privpool->volumes.objs, + privpool->volumes.count, privvol) < 0) goto cleanup; privpool->def->allocation += privvol->allocation; privpool->def->available = (privpool->def->capacity - privpool->def->allocation); - privpool->volumes.objs[privpool->volumes.count++] = privvol; - ret = virGetStorageVol(pool->conn, privpool->def->name, privvol->name, privvol->key, NULL, NULL); @@ -5624,18 +5612,7 @@ testStorageVolDelete(virStorageVolPtr vol, 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--; - + VIR_DELETE_ELEMENT(privpool->volumes.objs, i, privpool->volumes.count); break; } } -- 1.9.0
On 03/07/2014 02:46 AM, Michal Privoznik wrote:
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/test/test_driver.c | 41 +++++++++-------------------------------- 1 file changed, 9 insertions(+), 32 deletions(-)
ACK -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/virjson.c | 12 +++--------- src/util/virlockspace.c | 12 ++---------- 2 files changed, 5 insertions(+), 19 deletions(-) diff --git a/src/util/virjson.c b/src/util/virjson.c index 7044e11..02b5f3f 100644 --- a/src/util/virjson.c +++ b/src/util/virjson.c @@ -58,7 +58,7 @@ typedef virJSONParser *virJSONParserPtr; struct _virJSONParser { virJSONValuePtr head; virJSONParserStatePtr state; - unsigned int nstate; + size_t nstate; }; @@ -889,10 +889,7 @@ static int virJSONParserHandleEndMap(void *ctx) return 0; } - if (VIR_REALLOC_N(parser->state, - parser->nstate - 1) < 0) - return 0; - parser->nstate--; + VIR_DELETE_ELEMENT(parser->state, parser->nstate - 1, parser->nstate); return 1; } @@ -939,10 +936,7 @@ static int virJSONParserHandleEndArray(void *ctx) return 0; } - if (VIR_REALLOC_N(parser->state, - parser->nstate - 1) < 0) - return 0; - parser->nstate--; + VIR_DELETE_ELEMENT(parser->state, parser->nstate - 1, parser->nstate); return 1; } diff --git a/src/util/virlockspace.c b/src/util/virlockspace.c index cab7775..90a39bb 100644 --- a/src/util/virlockspace.c +++ b/src/util/virlockspace.c @@ -689,11 +689,7 @@ int virLockSpaceReleaseResource(virLockSpacePtr lockspace, goto cleanup; } - if (i < (res->nOwners - 1)) - memmove(res->owners + i, - res->owners + i + 1, - (res->nOwners - i - 1) * sizeof(res->owners[0])); - VIR_SHRINK_N(res->owners, res->nOwners, 1); + VIR_DELETE_ELEMENT(res->owners, i, res->nOwners); if ((res->nOwners == 0) && virHashRemoveEntry(lockspace->resources, resname) < 0) @@ -735,11 +731,7 @@ virLockSpaceRemoveResourcesForOwner(const void *payload, data->count++; - if (i < (res->nOwners - 1)) - memmove(res->owners + i, - res->owners + i + 1, - (res->nOwners - i - 1) * sizeof(res->owners[0])); - VIR_SHRINK_N(res->owners, res->nOwners, 1); + VIR_DELETE_ELEMENT(res->owners, i, res->nOwners); if (res->nOwners) { VIR_DEBUG("Other shared owners remain"); -- 1.9.0
On 03/07/2014 02:46 AM, Michal Privoznik wrote:
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/virjson.c | 12 +++--------- src/util/virlockspace.c | 12 ++---------- 2 files changed, 5 insertions(+), 19 deletions(-)
ACK -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/xen/xen_driver.c | 19 ++----------------- src/xen/xen_driver.h | 2 +- src/xen/xen_inotify.c | 13 ++----------- src/xen/xm_internal.c | 18 +++--------------- src/xen/xs_internal.c | 19 ++----------------- src/xen/xs_internal.h | 2 +- 6 files changed, 11 insertions(+), 62 deletions(-) diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index ee05fb4..1880b22 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -2879,13 +2879,9 @@ xenUnifiedAddDomainInfo(xenUnifiedDomainInfoListPtr list, info->id = id; /* Make space on list */ - n = list->count; - if (VIR_REALLOC_N(list->doms, n + 1) < 0) { + if (VIR_APPEND_ELEMENT(list->doms, list->count, info) < 0) goto error; - } - list->doms[n] = info; - list->count++; return 0; error: if (info) @@ -2915,18 +2911,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--; - + VIR_DELETE_ELEMENT(list->doms, i, list->count); return 0; } } diff --git a/src/xen/xen_driver.h b/src/xen/xen_driver.h index b8c1c27..54a1124 100644 --- a/src/xen/xen_driver.h +++ b/src/xen/xen_driver.h @@ -104,7 +104,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 2e9787f..a58509b 100644 --- a/src/xen/xen_inotify.c +++ b/src/xen/xen_inotify.c @@ -173,17 +173,8 @@ xenInotifyXendDomainsDirRemoveEntry(virConnectPtr conn, const char *fname) 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--; + VIR_DELETE_ELEMENT(priv->configInfoList->doms, i, + priv->configInfoList->count); return 0; } } diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c index d9a3502..8037b2e 100644 --- a/src/xen/xm_internal.c +++ b/src/xen/xm_internal.c @@ -1262,10 +1262,8 @@ xenXMDomainAttachDeviceFlags(virConnectPtr conn, case VIR_DOMAIN_DEVICE_NET: { - if (VIR_REALLOC_N(def->nets, def->nnets+1) < 0) + if (VIR_APPEND_ELEMENT(def->nets, def->nnets, dev->data.net) < 0) goto cleanup; - def->nets[def->nnets++] = dev->data.net; - dev->data.net = NULL; break; } @@ -1348,12 +1346,7 @@ xenXMDomainDetachDeviceFlags(virConnectPtr conn, 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--; + VIR_DELETE_ELEMENT(def->disks, i, def->ndisks); break; } } @@ -1365,12 +1358,7 @@ xenXMDomainDetachDeviceFlags(virConnectPtr conn, 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--; + VIR_DELETE_ELEMENT(def->nets, i, def->nnets); break; } } diff --git a/src/xen/xs_internal.c b/src/xen/xs_internal.c index ccdb736..2d02e6e 100644 --- a/src/xen/xs_internal.c +++ b/src/xen/xs_internal.c @@ -666,14 +666,9 @@ xenStoreAddWatch(virConnectPtr conn, VIR_STRDUP(watch->token, token) < 0) goto error; - /* Make space on list */ - n = list->count; - if (VIR_REALLOC_N(list->watches, n + 1) < 0) + if (VIR_APPEND_ELEMENT_COPY(list->watches, list->count, watch) < 0) goto error; - list->watches[n] = watch; - list->count++; - return xs_watch(priv->xshandle, watch->path, watch->token); error: @@ -719,17 +714,7 @@ xenStoreRemoveWatch(virConnectPtr conn, const char *path, const char *token) 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--; + VIR_DELETE_ELEMENT(list->watches, i, list->count); return 0; } } diff --git a/src/xen/xs_internal.h b/src/xen/xs_internal.h index bc5bd8c..24e2627 100644 --- a/src/xen/xs_internal.h +++ b/src/xen/xs_internal.h @@ -71,7 +71,7 @@ typedef struct _xenStoreWatch xenStoreWatch; typedef xenStoreWatch *xenStoreWatchPtr; struct _xenStoreWatchList { - unsigned int count; + size_t count; xenStoreWatchPtr *watches; }; typedef struct _xenStoreWatchList xenStoreWatchList; -- 1.9.0
On 03/07/2014 02:46 AM, Michal Privoznik wrote:
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/xen/xen_driver.c | 19 ++----------------- src/xen/xen_driver.h | 2 +- src/xen/xen_inotify.c | 13 ++----------- src/xen/xm_internal.c | 18 +++--------------- src/xen/xs_internal.c | 19 ++----------------- src/xen/xs_internal.h | 2 +- 6 files changed, 11 insertions(+), 62 deletions(-)
ACK-- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/xenxs/xen_sxpr.c | 43 +++++++++++++++++-------------------------- src/xenxs/xen_xm.c | 27 ++++++++------------------- 2 files changed, 25 insertions(+), 45 deletions(-) diff --git a/src/xenxs/xen_sxpr.c b/src/xenxs/xen_sxpr.c index d366b1b..1319c74 100644 --- a/src/xenxs/xen_sxpr.c +++ b/src/xenxs/xen_sxpr.c @@ -601,10 +601,9 @@ xenParseSxprNets(virDomainDefPtr def, VIR_STRDUP(net->model, "netfront") < 0) goto cleanup; - if (VIR_REALLOC_N(def->nets, def->nnets + 1) < 0) + if (VIR_APPEND_ELEMENT(def->nets, def->nnets, net) < 0) goto cleanup; - def->nets[def->nnets++] = net; vif_index++; } } @@ -685,12 +684,11 @@ xenParseSxprSound(virDomainDefPtr def, goto error; } - if (VIR_REALLOC_N(def->sounds, def->nsounds+1) < 0) { + if (VIR_APPEND_ELEMENT(def->sounds, def->nsounds, sound) < 0) { virDomainSoundDefFree(sound); goto error; } - def->sounds[def->nsounds++] = sound; offset = offset2 ? offset2 + 1 : NULL; } while (offset); } @@ -1057,10 +1055,8 @@ xenParseSxprPCI(virDomainDefPtr def, dev->source.subsys.u.pci.addr.slot = slotID; dev->source.subsys.u.pci.addr.function = funcID; - if (VIR_REALLOC_N(def->hostdevs, def->nhostdevs+1) < 0) + if (VIR_APPEND_ELEMENT(def->hostdevs, def->nhostdevs, dev) < 0) goto error; - - def->hostdevs[def->nhostdevs++] = dev; } return 0; @@ -1326,11 +1322,10 @@ xenParseSxpr(const struct sexpr *root, disk->bus = VIR_DOMAIN_DISK_BUS_IDE; disk->readonly = true; - if (VIR_REALLOC_N(def->disks, def->ndisks+1) < 0) { + if (VIR_APPEND_ELEMENT(def->disks, def->ndisks, disk) < 0) { virDomainDiskDefFree(disk); goto error; } - def->disks[def->ndisks++] = disk; } } @@ -1361,11 +1356,10 @@ xenParseSxpr(const struct sexpr *root, } disk->bus = VIR_DOMAIN_DISK_BUS_FDC; - if (VIR_REALLOC_N(def->disks, def->ndisks+1) < 0) { + if (VIR_APPEND_ELEMENT(def->disks, def->ndisks, disk) < 0) { virDomainDiskDefFree(disk); goto error; } - def->disks[def->ndisks++] = disk; } } } @@ -1395,13 +1389,12 @@ xenParseSxpr(const struct sexpr *root, virDomainChrDefPtr chr; if ((chr = xenParseSxprChar(tmp, tty)) == NULL) goto error; - if (VIR_REALLOC_N(def->serials, def->nserials+1) < 0) { - virDomainChrDefFree(chr); - goto error; - } chr->deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL; chr->target.port = def->nserials + ports_skipped; - def->serials[def->nserials++] = chr; + if (VIR_APPEND_ELEMENT(def->serials, def->nserials, chr) < 0) { + virDomainChrDefFree(chr); + goto error; + } } else ports_skipped++; @@ -1417,13 +1410,12 @@ xenParseSxpr(const struct sexpr *root, virDomainChrDefPtr chr; if ((chr = xenParseSxprChar(tmp, tty)) == NULL) goto error; - if (VIR_REALLOC_N(def->serials, def->nserials+1) < 0) { - virDomainChrDefFree(chr); - goto error; - } chr->deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL; chr->target.port = 0; - def->serials[def->nserials++] = chr; + if (VIR_APPEND_ELEMENT(def->serials, def->nserials, chr) < 0) { + virDomainChrDefFree(chr); + goto error; + } } } @@ -1433,13 +1425,12 @@ xenParseSxpr(const struct sexpr *root, /* XXX does XenD stuff parallel port tty info into xenstore somewhere ? */ if ((chr = xenParseSxprChar(tmp, NULL)) == NULL) goto error; - if (VIR_REALLOC_N(def->parallels, def->nparallels+1) < 0) { - virDomainChrDefFree(chr); - goto error; - } chr->deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_PARALLEL; chr->target.port = 0; - def->parallels[def->nparallels++] = chr; + if (VIR_APPEND_ELEMENT(def->parallels, def->nparallels, chr) < 0) { + virDomainChrDefFree(chr); + goto error; + } } } else if (def->id != 0) { if (VIR_ALLOC_N(def->consoles, 1) < 0) diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c index 3a57547..a70c5e3 100644 --- a/src/xenxs/xen_xm.c +++ b/src/xenxs/xen_xm.c @@ -608,10 +608,8 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, disk->shared = true; /* Maintain list in sorted order according to target device name */ - if (VIR_REALLOC_N(def->disks, def->ndisks+1) < 0) + if (VIR_APPEND_ELEMENT(def->disks, def->ndisks, disk) < 0) goto cleanup; - def->disks[def->ndisks++] = disk; - disk = NULL; skipdisk: list = list->next; @@ -637,10 +635,8 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, disk->bus = VIR_DOMAIN_DISK_BUS_IDE; disk->readonly = true; - if (VIR_REALLOC_N(def->disks, def->ndisks+1) < 0) + if (VIR_APPEND_ELEMENT(def->disks, def->ndisks, disk) < 0) goto cleanup; - def->disks[def->ndisks++] = disk; - disk = NULL; } } @@ -778,10 +774,8 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, VIR_STRDUP(net->ifname, vifname) < 0) goto cleanup; - if (VIR_REALLOC_N(def->nets, def->nnets+1) < 0) + if (VIR_APPEND_ELEMENT(def->nets, def->nnets, net) < 0) goto cleanup; - def->nets[def->nnets++] = net; - net = NULL; skipnic: list = list->next; @@ -869,12 +863,10 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, hostdev->source.subsys.u.pci.addr.slot = slotID; hostdev->source.subsys.u.pci.addr.function = funcID; - if (VIR_REALLOC_N(def->hostdevs, def->nhostdevs+1) < 0) { + if (VIR_APPEND_ELEMENT(def->hostdevs, def->nhostdevs, hostdev) < 0) { virDomainHostdevDefFree(hostdev); goto cleanup; } - def->hostdevs[def->nhostdevs++] = hostdev; - hostdev = NULL; skippci: list = list->next; @@ -1084,16 +1076,13 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, if (!(chr = xenParseSxprChar(port, NULL))) goto cleanup; - if (VIR_REALLOC_N(def->serials, def->nserials+1) < 0) { - virDomainChrDefFree(chr); - goto cleanup; - } - chr->deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL; chr->target.port = portnum; - def->serials[def->nserials++] = chr; - chr = NULL; + if (VIR_APPEND_ELEMENT(def->serials, def->nserials, chr) < 0) { + virDomainChrDefFree(chr); + goto cleanup; + } list = list->next; } -- 1.9.0
On 03/07/2014 02:46 AM, Michal Privoznik wrote:
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/xenxs/xen_sxpr.c | 43 +++++++++++++++++-------------------------- src/xenxs/xen_xm.c | 27 ++++++++------------------- 2 files changed, 25 insertions(+), 45 deletions(-)
ACK -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake -
Michal Privoznik