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))) >