Please use "error and out" logic (Re: [PATCH] Avoid privileged segfault in GetNCChanges and add many more tests)

[Ralph Böhme]

On Wed, Aug 09, 2017 at 03:26:29PM +0200, Ralph Böhme via samba-technical wrote:
> On Wed, Aug 09, 2017 at 09:10:22AM -0400, Simo via samba-technical wrote:
> > Thanks metze,
> > readability and modifiability of code are very important, and become
> > even more important as a project grows and ages.
> > 
> > It would be nice to add a style section to the docs that more
> > forcefully and clearly explain these subtle coding style issues.
> 
> patch attached.

updated version removing the unrelated hunk. Sorry!

Cheerio!
-slow
-------------- next part --------------
From 5c5101b09c9a884819ccf156ffd43f71ea3ae69c Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Wed, 9 Aug 2017 15:24:41 +0200
Subject: [PATCH] README.Coding: add "Error and out logic"

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 README.Coding | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/README.Coding b/README.Coding
index 19a363f..e89925c 100644
--- a/README.Coding
+++ b/README.Coding
@@ -445,6 +445,55 @@ The only exception is the test code that depends repeated use of calls
 like CHECK_STATUS, CHECK_VAL and others.
 
 
+Error and out logic
+-------------------
+
+Don't do this:
+
+	frame = talloc_stackframe();
+
+	if (ret == LDB_SUCCESS) {
+		if (result->count == 0) {
+			ret = LDB_ERR_NO_SUCH_OBJECT;
+		} else {
+			struct ldb_message *match =
+				get_best_match(dn, result);
+			if (match == NULL) {
+				TALLOC_FREE(frame);
+				return LDB_ERR_OPERATIONS_ERROR;
+			}
+			*msg = talloc_move(mem_ctx, &match);
+		}
+	}
+
+	TALLOC_FREE(frame);
+	return ret;
+
+It should be:
+
+	frame = talloc_stackframe();
+
+	if (ret != LDB_SUCCESS) {
+		TALLOC_FREE(frame);
+		return ret;
+	}
+
+	if (result->count == 0) {
+		TALLOC_FREE(frame);
+		return LDB_ERR_NO_SUCH_OBJECT;
+	}
+
+	match = get_best_match(dn, result);
+	if (match == NULL) {
+		TALLOC_FREE(frame);
+		return LDB_ERR_OPERATIONS_ERROR;
+	}
+
+	*msg = talloc_move(mem_ctx, &match);
+	TALLOC_FREE(frame);
+	return LDB_SUCCESS;
+
+
 DEBUG statements
 ----------------
 
-- 
2.9.4