Re: [libvirt] [PATCH 05/12] util: storage: Copy hosts of a storage file only if they exist
On 11/12/2014 08:47 AM, Peter Krempa wrote:
If there are no hosts for a storage source virStorageSourceCopy
would
try to copy them anyways. As the success of virStorageNetHostDefCopy is
determined by returning a pointer and malloc of 0 elements might return
NULL according to the implementation, the result of the copy function
may vary.
Fix this by copying the hosts array only if there are hosts defined.
---
src/util/virstoragefile.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
I see 'virStorageSourceNewFromBackingRelative' also has the same issue.
Could use this opportunity to fix that - or modify the API to adjust the
return and only do the alloc if source->nhosts > 0.
I also note that virStorageNetHostDefCopy is only used in
virstoragefile.c too - cause for statification?
ACK - I'll let you figure the best option... Definitely need a check in
both callers though.
John
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 8e9d115..c2d5b46 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1852,9 +1852,12 @@ virStorageSourceCopy(const virStorageSource *src,
VIR_STRDUP(ret->compat, src->compat) < 0)
goto error;
- if (!(ret->hosts = virStorageNetHostDefCopy(src->nhosts, src->hosts)))
- goto error;
- ret->nhosts = src->nhosts;
+ if (src->nhosts) {
+ if (!(ret->hosts = virStorageNetHostDefCopy(src->nhosts, src->hosts)))
+ goto error;
+
+ ret->nhosts = src->nhosts;
+ }
if (src->srcpool &&
!(ret->srcpool = virStorageSourcePoolDefCopy(src->srcpool)))