[libvirt] [PATCH revision RFC 2/2] lxc: Fix return values of veth.c functions - suggested changes

[Daniel Veillard]

On Thu, Jul 29, 2010 at 12:18:24PM -0400, Laine Stump wrote:
> Some suggested changes to your latest patch (I did the review by
> applying your patch, then examining the functions that were touched,
> focusing just on setting of rc)
> 
> Summary:
> 
> 1) virAsprintf() will return the number of characters in the new
>    string on success, not 0, so we need to only set rc if it fails
>    (< 0). Assigning rc on success causes the caller to falsely believe
>    the function failed.
> 
> 2) lxcVmCleanup was always doing the if (WIFEXITED() ...) even
>    if waitpid had failed. I don't know if the behavior of WIFEXITED
>    is defined if waitpid fails, but all the other uses I can find
>    avoid calling WIFEXITED and WEXITSTATUS if waitpid fails, so that's
>    what I did here.
> 
> 3) lxcSetupInterfaces - rather than explicitly setting rc from the
>    return of functions, since it defaults to -1, I just goto
>    error_exit if the functions return < 0. That's just cosmetic. The
>    real problem is that rc was being set from brAddInterface, which
>    returns > 0 on failure.
> 
> 4) assigning "rc = -1" at the beginning of each veth.c function is a
>    dead store, since it will always be set by the call to virRun(). This
>    causes coverity code analysis tool to report problems.
> ---
>  src/lxc/lxc_container.c |    6 ++++--
>  src/lxc/lxc_driver.c    |   19 ++++++-------------
>  src/lxc/veth.c          |   12 ++++++------
>  3 files changed, 16 insertions(+), 21 deletions(-)

  Okay, looks fine too, ACK,

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/