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

[Eric Blake]

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:

success:
   /* anything done only on success */
   ret = 0;
cleanup:
   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 at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120814/8b63a4d9/attachment-0001.sig>