Re: [libvirt] [PATCH 01/21] Added missing virBufferFreeAndReset on the query buffer to free some memory

On 10/09/2014 03:51 AM, Yves Vinter wrote: At this point, you can wait for more reviews on the rest of the patches. But yes, at some point, you'll want to send a v2 of the series with all the review changes incorporated. Not if you take my alternate proposed 1/21 patch. I'd rather fix the one function than have to fix every caller. It's bad to have a function that releases resources only some of the time. It's better to either ALWAYS release the resources, and document that the function consumes the argument, or to NEVER release the resources, leaving it up to the caller. Since the existing code is already semi-relying on the function consuming the argument (such as when a single hyperv_driver.c function reuses a query buffer twice in the same function, possible because the first use of the query was reset before building the second use of the buffer), my choice is to have the query parameter always consumed, even on error. Less code changes that way. It's okay, and even preferred, to interleave your replies (as I'm doing here), rather than top-posting. It makes it easier to follow the conversation when it is interleaved. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org