[PATCH] CodingStyle: Clarify and complete chapter 7

From: Jean Delvare
Date: Mon Jul 25 2016 - 08:29:21 EST


Chapter 7 (Centralized exiting of functions) of the coding style
documentation is unclear at times, and lacks some information (such
as the possibility to indent labels with a single space.) Clarify and
complete it.

Signed-off-by: Jean Delvare <jdelvare@xxxxxxx>
Cc: Markus Elfring <elfring@xxxxxxxxxxxxxxxxxxxxx>
Cc: Jonathan Corbet <corbet@xxxxxxx>
---
Documentation/CodingStyle | 25 +++++++++++++++++++------
1 file changed, 19 insertions(+), 6 deletions(-)

--- linux-4.7-rc7.orig/Documentation/CodingStyle 2016-07-04 08:01:00.000000000 +0200
+++ linux-4.7-rc7/Documentation/CodingStyle 2016-07-25 14:25:12.498328631 +0200
@@ -396,9 +396,13 @@ locations and some common work such as c
cleanup needed then just return directly.

Choose label names which say what the goto does or why the goto exists. An
-example of a good name could be "out_buffer:" if the goto frees "buffer". Avoid
-using GW-BASIC names like "err1:" and "err2:". Also don't name them after the
-goto location like "err_kmalloc_failed:"
+example of a good name could be "out_free_buffer:" if the goto frees "buffer".
+Avoid using GW-BASIC names like "err1:" and "err2:", as you would have to
+renumber them if you ever add or remove exit paths, and they make correctness
+difficult to verify anyway.
+
+It is advised to indent labels with a single space (not tab), so that
+"diff -p" does not confuse labels with functions.

The rationale for using gotos is:

@@ -425,20 +429,29 @@ The rationale for using gotos is:
goto out_buffer;
}
...
- out_buffer:
+ out_free_buffer:
kfree(buffer);
return result;
}

A common type of bug to be aware of is "one err bugs" which look like this:

- err:
+ err:
kfree(foo->bar);
kfree(foo);
return ret;

The bug in this code is that on some exit paths "foo" is NULL. Normally the
-fix for this is to split it up into two error labels "err_bar:" and "err_foo:".
+fix for this is to split it up into two error labels "err_free_bar:" and
+"err_free_foo:":
+
+ err_free_bar:
+ kfree(foo->bar);
+ err_free_foo:
+ kfree(foo);
+ return ret;
+
+Ideally you should simulate errors to test all exit paths.


Chapter 8: Commenting


--
Jean Delvare
SUSE L3 Support