[libvirt] [PATCH revision RFC 2/2] lxc: Fix return values of veth.c functions - suggested changes
veillard at redhat.com
Thu Jul 29 17:29:54 UTC 2010
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)
> 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 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/
More information about the libvir-list