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

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

 



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@xxxxxxxxxx    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

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