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

[Eric Blake]

On 10/08/2014 06:33 AM, Yves Vinter wrote:
> From: yvinter <yves.vinter at bull.net>

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.

> +++ b/src/hyperv/hyperv_driver.c
> @@ -201,6 +201,7 @@ hypervConnectOpen(virConnectPtr conn, virConnectAuthPtr auth, unsigned int flags
>      VIR_FREE(username);
>      VIR_FREE(password);
>      hypervFreeObject(priv, (hypervObject *)computerSystem);
> +    virBufferFreeAndReset(&query);

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 539 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20141008/6ea8628c/attachment-0001.sig>