linux-kernel - [PATCH] CodingStyle: Clarify and complete chapter 7

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 25 Jul 2016 14:29:06 +0200
From:	Jean Delvare <jdelvare@...e.de>
To:	linux-doc@...r.kernel.org
Cc:	linux-kernel@...r.kernel.org,
	Markus Elfring <elfring@...rs.sourceforge.net>,
	Jonathan Corbet <corbet@....net>
Subject: [PATCH] CodingStyle: Clarify and complete chapter 7

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@...e.de>
Cc: Markus Elfring <elfring@...rs.sourceforge.net>
Cc: Jonathan Corbet <corbet@....net>
---
 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