On 2012年07月09日 21:21, Peter Krempa wrote:

--- src/conf/storage_conf.c | 12 ++++++------ src/storage/storage_backend_fs.c | 3 +-- 2 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index bf4567f..ab8df9e 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -593,19 +593,19 @@ virStoragePoolDefParseSourceString(const char *srcSpec, xmlXPathContextPtr xpath_ctxt = NULL; virStoragePoolSourcePtr def = NULL, ret = NULL;

- if (!(doc = virXMLParseStringCtxt(srcSpec, _("(storage_source_specification)"),&xpath_ctxt))) { + if (!(doc = virXMLParseStringCtxt(srcSpec, + _("(storage_source_specification)"), +&xpath_ctxt))) goto cleanup; - }

if (VIR_ALLOC(def)< 0) { virReportOOMError(); goto cleanup; }

- node = virXPathNode("/source", xpath_ctxt); - if (!node) { - virStorageReportError(VIR_ERR_XML_ERROR, - "%s", _("root element was not source")); + if (!(node = virXPathNode("/source", xpath_ctxt))) { + virStorageReportError(VIR_ERR_XML_ERROR, "%s", + _("root element was not source")); goto cleanup; }

diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 87d9192..c736496 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -190,8 +190,7 @@ virStorageBackendFileSystemNetFindPoolSourcesFunc(virStoragePoolObjPtr pool ATTR

path = groups[0];

- name = strrchr(path, '/'); - if (name == NULL) { + if (!(name = strrchr(path, '/'))) { virStorageReportError(VIR_ERR_INTERNAL_ERROR, _("invalid netfs path (no /): %s"), path); goto cleanup;

ACK

On 2012年07月09日 21:21, Peter Krempa wrote:

As the storage pool sources are stored in a list of structs, the pointer returned by virStoragePoolSourceListNewSource() shouldn't be freed as it points in the middle of a memory block. This combined with a regression that takes the error path every time on caused a double-free abort on the src struct in question. --- src/storage/storage_backend_fs.c | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index c736496..5e3da14 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -218,10 +218,8 @@ virStorageBackendFileSystemNetFindPoolSourcesFunc(virStoragePoolObjPtr pool ATTR } src->format = VIR_STORAGE_POOL_NETFS_NFS;

- src = NULL; ret = 0; cleanup: - virStoragePoolSourceFree(src); return ret; }

ACK

On 2012年07月09日 21:21, Peter Krempa wrote:

Commit 122fa379de44a2fd0a6d5fbcb634535d647ada17 introduces option to store more than one host entry in a storage pool source definition. That commit causes a regression, where a check is added that only one host entry should be present (that actualy is not present as the source structure was just allocated and zeroed) instead of allocating memory for the host entry. --- src/storage/storage_backend_fs.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 5e3da14..5eb486e 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -205,11 +205,11 @@ virStorageBackendFileSystemNetFindPoolSourcesFunc(virStoragePoolObjPtr pool ATTR if (!(src = virStoragePoolSourceListNewSource(&state->list))) goto cleanup;

- if (src->nhost != 1) { - virStorageReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Expected exactly 1 host for the storage pool")); + if (VIR_ALLOC_N(src->hosts, 1)< 0) { + virReportOOMError(); goto cleanup; } + src->nhost = 1;

if (!(src->hosts[0].name = strdup(state->host)) || !(src->dir = strdup(path))) {

ACK