[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [RFC] virCommandRun return error number



On 11.04.2013 15:48, harryxiyou wrote:
> On Thu, Apr 11, 2013 at 9:38 PM, Michal Privoznik <mprivozn redhat com> wrote:
> [...]
>> Maybe you've pasted wrong code snippet, but the one you've pasted is
>> correct. If virCommandRun() fails and returns -1; we return that value
>> to the caller. And if the command doesn't fail, we call
>> virStorageBackendSheepdogParseNodeInfo to parse output of the command.
>>
> 
> Sorry, i have asked for a stupid bug. Let us check following one.
> 
> [...]
> 114 static int
> 115 virStorageBackendSheepdogRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED,
> 116                                      virStoragePoolObjPtr pool)
> 117 {
> 118     int ret;
> 119     char *output = NULL;
> 120     virCommandPtr cmd;
> 121
> 122     cmd = virCommandNewArgList(COLLIE, "node", "info", "-r", NULL);
> 123     virStorageBackendSheepdogAddHostArg(cmd, pool);
> [...]
> 
> virCommandNewArgList may return ‘NULL’, so i think we should check
> this condition, which we need not do following stuffs, right?
> 

And yet again - the code is bugfree. If virCommandNewArgList returns
NULL, this is passed to virStorageBackendSheepdogAddHostArg() which
operates then with virCommand* API. This API is perfectly comfortable
with working over NULL. That is, so we can have:

cmd = virCommmandNew*(blah, blah, ....);
virCommand*(cmd, ...); /* repeated multiple times */
if (virCommandRun(cmd, NULL) < 0) {
    printf("Ouch!");
    goto cleanup;
}

instead of:

cmd = virCommandNew*(blah, blah, ...);
if (!cmd)
    goto cleanup;

if (virCommand*(cmd, ...) < 0)
    goto cleanup;

if (virCommand*(cmd, ...) < 0)
    goto cleanup;

if (virCommand*(cmd, ...) < 0)
    goto cleanup;

if (virCommandRun(cmd, ...) < 0) {
    printf("Ouch!");
    goto cleanup;
}


If you want to dig into libvirt code I'd recommend going though our HACKING:

http://libvirt.org/hacking.html

and generating tags for the code (ctags -R .) so whenever you open a
file in your editor you can easily jump to a function you cursor is at.
That is: in this case, if you run with cursor over the line number 123
and press CTRL + ] in vim (I bet there are plenty of emacs users who
will contribute with their hotkey for this) vim takes you to the
function definition so you can see what it actually does. And if you
apply this process twice it will get you to the virCommand*
implementation where you'll see NULL check at the beginning of each API
(if not, *that's* a real bug).

Happy hacking!

Michal


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]