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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]