[libvirt] [PATCH 05/12] util: storage: Copy hosts of a storage file only if they exist

[Peter Krempa]

On 11/20/14 16:10, John Ferlan wrote:
> 
> 
> 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)))

Avoiding the allocation would inhibit checking for failed allocation.
This would require to rewrite it to return an int and return the
allocated array as an argument. I'll stick with adding an explicit check
for now.

>> +            goto error;
>> +
>> +        ret->nhosts = src->nhosts;
>> +    }
>>
>>      if (src->srcpool &&
>>          !(ret->srcpool = virStorageSourcePoolDefCopy(src->srcpool)))
>>


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20141120/77695f80/attachment-0001.sig>