Re: [libvirt] [PATCHv2 05/13] snapshot: indent domain xml when nesting
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(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org