[libvirt] [PATCH] vbox: fix VIR_STRDUP value check

[Ján Tomko]

On 05/20/2013 05:55 PM, Eric Blake wrote:
> On 05/20/2013 03:59 AM, Ján Tomko wrote:
>> In my review of 31532ca I missed the fact that VIR_STRDUP
>> now returns 1 on success, and 0 if the source was NULL.
>>
>> (This still doesn't add proper OOM error handling.)
>> ---
>>  src/vbox/vbox_tmpl.c | 12 ++++++------
>>  1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
>> index eb8ac63..163aeff 100644
>> --- a/src/vbox/vbox_tmpl.c
>> +++ b/src/vbox/vbox_tmpl.c
>> @@ -2688,9 +2688,9 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) {
>>  
>>                  if (hddType == HardDiskType_Immutable)
>>                      def->disks[hddNum]->readonly = true;
>> -                if (VIR_STRDUP_QUIET(def->disks[hddNum]->src, hddlocation) == 0 &&
>> -                    VIR_STRDUP_QUIET(def->disks[hddNum]->dst, "hdd") == 0)
>> -                    hddNum++;
>> +                ignore_value(VIR_STRDUP(def->disks[hddNum]->src, hddlocation));
>> +                ignore_value(VIR_STRDUP(def->disks[hddNum]->dst, "hdd"));
>> +                hddNum++;
> 
> This changes the situations in which hddNum is set; now hddNum is
> incremented even if you hit an OOM error.  Is that really what you want?
> 

It's what was done before 31532ca and handling an OOM error properly would
require rewriting the whole function.

And in this case, the increment doesn't matter at all, since hddNum is never
used again.

Jan