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

Re: [libvirt] [PATCHv3 4/5] network: change cleanup: to success/cleanup/error: in network*() functions

On 08/14/2012 04:17 PM, Laine Stump wrote:
> A later patch will be adding a counter that will be
> incremented/decremented each time an guest interface starts/stops
> using a particular network. For this to work, all types of networks
> need to go through a common return sequence rather than returning
> early. To setup for this, the existing a new success: label is added

s/the existing//

> (when necessary), a new error: label is added which does any cleanup
> necessary only for error returns and then does goto cleanup, and early
> returns are changed to goto error if it's a failure, or goto success
> if it's successful. This way the intent of all the gotos is
> unambiguous, and a successful return path never encounters the
> "error:" label.
> ---
> Change from v1: at Eric's suggestion, instead of having a cleanup label jumped to on success, and an inline "error" label jumped to on failure:

> I modified these function so that they have three labels:
>     success:
>        /* anything only done on success */
>        ret = 0;
>     cleanup:
>        /* common cleanup */
>        return ret;
>     error:
>        /* anything only done on error */
>        goto cleanup;

Yep, that style is fine with me.  An alternative style (but no need to
respin now) would be:

   /* anything done only on success */
   ret = 0;
   if (ret < 0) {
       /* anything only done on error */
   return ret;

but while it is fewer labels and no backwards goto, it loses out on the
nice 'goto error' naming, so you are just trading one style for another
with no real benefit.

> This has the dual advantage of making the intent of all gotos
> painfully clear, as well as not confusing anyone by having the control
> flow of a successful exit go past the error: label.
>  src/network/bridge_driver.c | 124 ++++++++++++++++++++++++--------------------
>  1 file changed, 69 insertions(+), 55 deletions(-)

ACK as-is.

Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

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