[libvirt] [PATCH 2/4] tests: Fix leaks in commandtest

[Eric Blake]

On 12/06/2010 05:44 AM, Jiri Denemark wrote:
>>>      free(virtTestLogContentAndReset());
>>>      cmd = virCommandNew(abs_builddir "/commandhelper-doesnotexist");
>>> -    if (virCommandRun(cmd, NULL) == 0)
>>> +    if (!cmd || virCommandRun(cmd, NULL) == 0)
>>
>> The API explicitly does *not* require you to check
>> !cmd after virCommandNew. virCommandRun() and other
>> APis will check that for you and return ENOMEM.
> 
> That was my suspicion too but then I looked at virCommandRun implementation
> and saw this:
> 
>     if (!cmd || cmd->has_error == -1) {
>         virCommandError(VIR_ERR_INTERNAL_ERROR, "%s",
>                         _("invalid use of command API"));
>         return -1;
>     }
>     if (cmd->has_error == ENOMEM) {
>         virReportOOMError();
>         return -1;
>     }
> 
> That is, if virCommandNew() returns NULL for either reason, by running
> virCommandRun() on it, the original error (probably OOM) gets overwritten by
> an internal error.

Oh, I see.  The correct fix is to swap that precedence in command.c (I'm
working on a patch):

if (!cmd || cmd->has_error == ENOMEM) {
    virReportOOMError();
    return -1;
}
if (cmd->has_error) {
    report invalid use of command as internal error
}

Then you can safely ignore allocation failure on virCommandNew up until
virCommandRun complains about the OOM failure, and it won't be treated
as an internal usage error.

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

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