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

On 10/08/2014 06:33 AM, Yves Vinter wrote: Your git config is incorrect; your email headers list your full name 'Yves Vinter' but this line means that git is trying to attribute to the authorship to your username 'yvinter'. We prefer that the git history include a full legal name rather than a username abbreviation. Long subject line; the first line of a commit message should generally be no more than about 60 characters, so that 'git log --oneline -30' still fits the information comfortably in an 80-column screen. Also, it is nice to include a 'topic:' lead-in. Ideally, the one-line summary is the "what", and then the rest of the commit message after a blank line is the "why". So I suggest: hyperv: avoid memory leaks Add missing virBufferFreeAndReset on query buffers used throughout the hyperv code. as well as mention any valgrind testing you did to prove that the leaks are fixed. Hmmm. query is initialized empty, until it is used in this code a few lines earlier: virBufferAddLit(&query, MSVM_COMPUTERSYSTEM_WQL_SELECT); virBufferAddLit(&query, "where "); virBufferAddLit(&query, MSVM_COMPUTERSYSTEM_WQL_PHYSICAL); if (hypervGetMsvmComputerSystemList(priv, &query, &computerSystem) < 0) { goto cleanup; but tracing through that code, hypervGetMsvmComputerSystemList in hyperv_wmi.generated.c calls hypervEnumAndPull in hyperv_wmi.c, which in turn calls virBufferContentAndReset(query) on the success path, leaving nothing to clean up here. Okay, I see where hypervEnumAndPull can return early without cleaning query; I wonder if it would have been better to patch THAT function to guarantee that the buffer is always clean on return, rather than having to patch every single caller. In fact, after looking through the entire patch, it looks like EVERY single addition of virBufferFreeAndReset(&query) is only ever useful in any case where hypervEnumAndPull returns early. I'm going to reply to this mail with an alternative shorter patch. I don't have access to hyperv to test it under an actual valgrind run, but it compiled for me. Can you please run it through your test setup to see if it solves the issues you were initially trying to address? -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org