Please use "error and out" logic (Re: [PATCH] Avoid privileged segfault in GetNCChanges and add many more tests)
Ralph Böhme
slow at samba.org
Wed Aug 9 13:26:29 UTC 2017
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.
Cheerio!
-slow
-------------- next part --------------
From 7e2fa6d9c53a0d80b2f1b6c0b867ccbcb5e0716b 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 +++++++++++++++++++++++++++++++++++++++++++++++++
STEPS.org | 1 -
2 files changed, 49 insertions(+), 1 deletion(-)
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
----------------
diff --git a/STEPS.org b/STEPS.org
index 1986640..cb532fe 100644
--- a/STEPS.org
+++ b/STEPS.org
@@ -89,7 +89,6 @@ In *struct share_mode_data* and *struct share_mode_entry*:
- bool *shared_persistent* in share_mode_entry
See [[Design][Design]] for an explenation.
-
* Implementation
** SMB layer
*** Flag checks in SMB2_CREATE_DURABLE_HANDLE_RECONNECT_V2 (MS-SMB2 3.3.5.9.12)
--
2.9.4
More information about the samba-technical
mailing list