Re: [libvirt] [PATCH 1/1] The use of goto

* 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 +