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

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(-)