On Fri, Mar 05, 2010 at 12:25:19PM -0500, David Allan wrote: > * Added a section on the appropriate and inappropriate uses of goto to the HACKING document and website. > --- > HACKING | 32 ++++++++++++++++++++++++++++++++ > docs/hacking.html.in | 41 +++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 73 insertions(+), 0 deletions(-) > > diff --git a/HACKING b/HACKING > index be8725d..db99630 100644 > --- a/HACKING > +++ b/HACKING > @@ -362,6 +362,38 @@ their jobs and cross-check format strings with the number and types > of arguments. > > > +Use of goto > +=========== > + > +The use of goto is not forbidden, and goto is widely used throughout > +libvirt. While the uncontrolled use of goto will quickly lead to > +unmaintainable code, there is a place for it in well structured code > +where its use increases readability and maintainability. > + > +The typical use of goto is to jump to cleanup code in the case of a > +long list of actions, any of which may fail and cause the entire > +operation to fail. In this case, a function will have a single label > +at the end of the funcion. It's almost always ok to use this style. > +In particular, if the cleanup code only involves free'ing memory, then > +having multiple labels is overkill. VIR_FREE() and every function > +named XXXFree() in libvirt is required to handle NULL as its arg. > +Thus you can safely call free on all the variables even if they were > +not yet allocated (yes they have to have been initialized to NULL). > +This is much simpler and clearer than having multiple labels. > + > +There are a couple of signs that a particular use of goto is not ok. > +The first is the use of multiple labels. If you find yourself using > +multiple labels, you're strongly encouraged to rework your code to > +eliminate all but one of them. The second is jumping back up, above > +the current line of code being executed. Please use some combination > +of looping constructs to re-execute code instead; it's almost > +certainly going to be more understandable by others. > > +Although libvirt does not encourage the Linux kernel wind/unwind style > +of multiple labels, there's a good general discussion of the issue at: > + > +http://kerneltrap.org/node/553/2131 > + Thanks, it would be good if we documented a consistent naming scheme for labels too. error: A path only taken upon return with an error code cleanup: A path taken upon return with success code + optional error no_memory: A path only taken upon return with an OOM error code retry: If needing to jump upwards (eg retry on EINTR) There are plenty of violations of these 4 names, but they are the most common, so we might want to convert others to use these names, or avoid goto if it wasn't appropriate. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list