[libvirt] [PATCHv3] lxc: Fix return values of veth.c functions - complete

[Laine Stump]

  On 07/29/2010 01:53 PM, Ryota Ozaki wrote:
> On Fri, Jul 30, 2010 at 2:32 AM, Laine Stump<laine at laine.org>  wrote:
>> From: Ryota Ozaki<ozaki.ryota at gmail.com>
>>
>> Previously, the functions in src/lxc/veth.c could sometimes return
>> positive values on failure rather than -1. This made accurate error
>> reporting difficult, and led to one failure to catch an error in a
>> calling function.
>>
>> This patch makes all the functions in veth.c consistently return 0 on
>> success, and -1 on failure. It also fixes up the callers to the veth.c
>> functions where necessary.
>>
>> Note that this patch may be related to the bug:
>>
>>   https://bugzilla.redhat.com/show_bug.cgi?id=607496.
>>
>> It will not fix the bug, but should unveil what happens.
>>
>> * po/POTFILES.in - add veth.c, which previously had no translatable strings
>> * src/lxc/lxc_controller.c
>> * src/lxc/lxc_container.c
>> * src/lxc/lxc_driver.c    - fixup callers to veth.c, and remove error logs,
>>                             as they are now done in veth.c
>> * src/lxc/veth.c - make all functions consistently return -1 on error.
>> * src/lxc/veth.h - use ATTRIBUTE_NONNULL to protect against NULL args.
>> ---
>> Changes from Ozaki's original patch:
>>
>> 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. Also changed to return -1 if waitpid fails or
>>    WEXITSTATUS != 0.
>>
>> 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 (note error log of errno from brAddInterface
>>    fixed in v3).
>>
>> 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.
> ACK. Thanks, Laine!

Pushed.