[libvirt] [PATCHv2 05/13] snapshot: indent domain xml when nesting

Eric Blake eblake at redhat.com
Thu Oct 20 22:29:17 UTC 2011


On 10/19/2011 10:04 PM, Hai Dong Li wrote:
> On 10/20/2011 03:08 AM, Peter Krempa wrote:
>> Dňa 29.9.2011 18:22, Eric Blake wrote / napísal(a):
>>> <domainsnapshot> is the first public instance of<domain> being
>>> used as a sub-element, although we have two other private uses
>>> (runtime state, and migration cookie). Although indentation has
>>> no effect on XML parsing, using it makes the output more consistent.
>>>

>>> @@ -431,10 +434,14 @@ static char
>>> *qemuMigrationCookieXMLFormatStr(qemuMigrationCookiePtr mig)
>>> {
>>> virBuffer buf = VIR_BUFFER_INITIALIZER;
>>>
>>> - qemuMigrationCookieXMLFormat(&buf, mig);
>>> + if (qemuMigrationCookieXMLFormat(&buf, mig)< 0) {
>>> + virBufferFreeAndReset(&buf);
>>> + return NULL;
>>> + }
>>>
>>> if (virBufferError(&buf)) {
>>> virReportOOMError();
>>> + virBufferFreeAndReset(&buf);
>> Probably shouldn't be necessary as the virBufferSetError already frees
>> the internal buffer.
> Maybe for sure. Relying on other function's error handling(especially
> when it is a deep calling line) is a little not that sure.

Peter's right that it wasn't strictly necessary, given the fact that any 
OOM error with virBuffer frees memory at that point, so there is nothing 
further to free here.  And Hai's right that relying on an obscure 
feature of the current implementation of the called function is harder 
to maintain than explicitly cleaning up after ourselves here, even if 
our cleanup happens to be a no-op due to the called function.  I kept 
this line as-is, considering that the failure case is unexpected in the 
first place, so the efficiency of avoiding a no-op is much less 
important than robustness if the virBuffer implementation changes in the 
future.

>>> return NULL;
>>> }
>> ACK,

Now pushed.

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org




More information about the libvir-list mailing list