[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 + Libvirt commiters guidelines ============================ diff --git a/docs/hacking.html.in b/docs/hacking.html.in index 788a49b..e10d2dd 100644 --- a/docs/hacking.html.in +++ b/docs/hacking.html.in @@ -389,6 +389,47 @@ of arguments. </p> + <h2><a name="goto">Use of goto</a></h2> + + <p> + 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. + </p> + + <p> + 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. + </p> + + <p> + 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. + </p> + + <p> + Although libvirt does not encourage the Linux kernel wind/unwind + style of multiple labels, there's a good general discussion of + the issue archived at + <a href=http://kerneltrap.org/node/553/2131>KernelTrap</a> + </p> <h2><a name="committers">Libvirt commiters guidelines</a></h2> -- 1.6.5.5