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@xxxxxxxxxx +1-801-349-2682
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list