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

Re: [libvirt] [PATCH 7/8] conf: check for buffer errors before virBufferUse



>>
>>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>>> index 7728321cb..4dc49fdb0 100644
>>> --- a/src/conf/domain_conf.c
>>> +++ b/src/conf/domain_conf.c
>>
>> [...]
>>
>>> @@ -23309,6 +23321,10 @@ static int
>>> virDomainPanicDefFormat(virBufferPtr buf,
>>>      virBufferAdjustIndent(&childrenBuf, indent + 2);
>>>      if (virDomainDeviceInfoFormat(&childrenBuf, &def->info, 0) < 0)
>>>          return -1;
>>> +
>>> +    if (virBufferCheckError(&childrenBuf) < 0)
>>> +        return -1;
>>> +
>>
>> Seeing a virBufferFreeAndReset below this if condition first had me
>> thinking, well that's unnecessary; however, in actuality I think we have
>> a leak any time virBufferUse doesn't return a non zero value call
>> virBufferAddBuffer to consume the buffer.
> 
> I do not see the leak. If we made no attempt at all to use the buffer,
> nothing should have been allocated. If we tried to add something to it,
> and failed on OOM, virBufferSetError should free the content and set use
> to zero. The only possible leak would be when we try to extend the
> buffer without actually writing any content to it - but we have no
> reason to do that in an XML file, since there's always going to be
> at least the element name.
> 
> Jan
> 

Hmm.. right - I guess it's seeing the FreeAndReset in some places and
not others that got me to thinking about this and of course being
somehow convinced that there could be a leak.  Perhaps those places
where FreeAndReset is called unnecessarily could be cleaned up (they're
not wrong, but they do nothing as long as the AddBuffer was used).

John


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