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 :|