[Patch] Fix the ";binary" suffix behavior

Volker Lendecke vl at samba.org
Tue Aug 9 04:08:04 UTC 2016


On Mon, Aug 08, 2016 at 05:13:59PM -0700, Jeremy Allison wrote:
> On Mon, Aug 08, 2016 at 06:00:00PM +0200, Niklas Abel wrote:
> 
> > No problem, thanks for the review and the fast response.
> 
> OK - LGTM. Can I get a second Team reviewer ?

Attached find a set of cosmetic patches on top. Feel free to comment &
squash.

On the semantics -- RFC4522 describes what this is supposed to do. Can
we really just ignore this?

Volker
-------------- next part --------------
>From 616f093e8bf56599f45db090579f2d3c197f35f3 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 9 Aug 2016 05:59:05 +0200
Subject: [PATCH 1/6] make str_list_copy_const_clean_suffix static

Otherwise we get an undefined prototype warning
---
 source4/dsdb/samdb/ldb_modules/resolve_oids.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/source4/dsdb/samdb/ldb_modules/resolve_oids.c b/source4/dsdb/samdb/ldb_modules/resolve_oids.c
index 3b587ba..ee40a94 100644
--- a/source4/dsdb/samdb/ldb_modules/resolve_oids.c
+++ b/source4/dsdb/samdb/ldb_modules/resolve_oids.c
@@ -475,8 +475,8 @@ const static char * strip_suffix (const void *mem_ctx, const char *attr, const c
  * Modified version of str_list_copy_const() which creates the new list without
  * entries with a ";binary" tail.
 */
-const char **str_list_copy_const_clean_suffix(TALLOC_CTX *mem_ctx,
-					  const char **list)
+static const char **str_list_copy_const_clean_suffix(TALLOC_CTX *mem_ctx,
+						     const char **list)
 {
 	int i;
 	const char **ret;
-- 
2.1.4


>From 8f842a7cb41d664ca229b82906aa75bd9354f52e Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 9 Aug 2016 05:59:57 +0200
Subject: [PATCH 2/6] 80-chars per line

---
 source4/dsdb/samdb/ldb_modules/resolve_oids.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/source4/dsdb/samdb/ldb_modules/resolve_oids.c b/source4/dsdb/samdb/ldb_modules/resolve_oids.c
index ee40a94..90986e6 100644
--- a/source4/dsdb/samdb/ldb_modules/resolve_oids.c
+++ b/source4/dsdb/samdb/ldb_modules/resolve_oids.c
@@ -444,7 +444,8 @@ static int resolve_oids_callback(struct ldb_request *req, struct ldb_reply *ares
  * Strips suffix from an attribute,
  * if there is any.
 */
-const static char * strip_suffix (const void *mem_ctx, const char *attr, const char *suffix)
+const static char *strip_suffix(const void *mem_ctx, const char *attr,
+				const char *suffix)
 {
 	size_t attr_length = 0;
 	size_t suffix_length = 0;
@@ -560,9 +561,10 @@ static int resolve_oids_search(struct ldb_module *module, struct ldb_request *re
 	}
 
 	if (needclean) {
-		cleaned_attrs = str_list_copy_const_clean_suffix(req,
-						discard_const_p(const char *, req->op.search.attrs));
-		if(cleaned_attrs) {
+		cleaned_attrs = str_list_copy_const_clean_suffix(
+			req,
+			discard_const_p(const char *, req->op.search.attrs));
+		if (cleaned_attrs) {
 			req->op.search.attrs = cleaned_attrs;
 		}
 	}
-- 
2.1.4


>From 52ebbfc2b4037300c40c0c8ece6eb8eb570a46e1 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 9 Aug 2016 06:02:10 +0200
Subject: [PATCH 3/6] convert a global variable to a local one

---
 source4/dsdb/samdb/ldb_modules/resolve_oids.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/source4/dsdb/samdb/ldb_modules/resolve_oids.c b/source4/dsdb/samdb/ldb_modules/resolve_oids.c
index 90986e6..454cb8d 100644
--- a/source4/dsdb/samdb/ldb_modules/resolve_oids.c
+++ b/source4/dsdb/samdb/ldb_modules/resolve_oids.c
@@ -451,7 +451,7 @@ const static char *strip_suffix(const void *mem_ctx, const char *attr,
 	size_t suffix_length = 0;
 	size_t new_attr_size = 0;
 	const char *tmp = NULL;
-	const static char *result = NULL;
+	const char *result = NULL;
 
 	if (!attr || !*attr || !suffix || !*suffix) {
 		return attr;
-- 
2.1.4


>From e9d624362b7d14663697cf649d0df5b7c60f5a0b Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 9 Aug 2016 06:03:14 +0200
Subject: [PATCH 4/6] unify talloc handling

Without this patch, our talloc hierarchy in the copy is a mix of old and new.
This will create problems once this function is used outside this very specific
use case.
---
 source4/dsdb/samdb/ldb_modules/resolve_oids.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/source4/dsdb/samdb/ldb_modules/resolve_oids.c b/source4/dsdb/samdb/ldb_modules/resolve_oids.c
index 454cb8d..9c78582 100644
--- a/source4/dsdb/samdb/ldb_modules/resolve_oids.c
+++ b/source4/dsdb/samdb/ldb_modules/resolve_oids.c
@@ -451,10 +451,9 @@ const static char *strip_suffix(const void *mem_ctx, const char *attr,
 	size_t suffix_length = 0;
 	size_t new_attr_size = 0;
 	const char *tmp = NULL;
-	const char *result = NULL;
 
 	if (!attr || !*attr || !suffix || !*suffix) {
-		return attr;
+		return talloc_strdup(mem_ctx, attr);
 	}
 	attr_length = strlen(attr);
 	suffix_length = strlen(suffix);
@@ -464,12 +463,9 @@ const static char *strip_suffix(const void *mem_ctx, const char *attr,
 	new_attr_size = (attr_length - suffix_length);
 	tmp = attr + new_attr_size;
 	if (strcasecmp(suffix, tmp) == 0) {
-		result = talloc_strndup(mem_ctx, attr, new_attr_size);
-		if (result) {
-			return result;
-		}
+		return talloc_strndup(mem_ctx, attr, new_attr_size);
 	}
-	return attr;
+	return talloc_strdup(mem_ctx, attr);
 }
 
 /**
-- 
2.1.4


>From 08417fddec2bf10fbb76c3ea1c51b0b9ca95fbec Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 9 Aug 2016 06:04:57 +0200
Subject: [PATCH 5/6] fix OOM handling

---
 source4/dsdb/samdb/ldb_modules/resolve_oids.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/source4/dsdb/samdb/ldb_modules/resolve_oids.c b/source4/dsdb/samdb/ldb_modules/resolve_oids.c
index 9c78582..5ba8475 100644
--- a/source4/dsdb/samdb/ldb_modules/resolve_oids.c
+++ b/source4/dsdb/samdb/ldb_modules/resolve_oids.c
@@ -490,7 +490,8 @@ static const char **str_list_copy_const_clean_suffix(TALLOC_CTX *mem_ctx,
 	for (i=0;list && list[i];i++) {
 		ret[i] = strip_suffix(mem_ctx, list[i], ";binary");
 		if (ret[i] == NULL) {
-			break;
+			TALLOC_FREE(ret);
+			return NULL;
 		}
 	}
 	ret[i] = NULL;
-- 
2.1.4


>From 8aa81d0df39e18be77ee2fca24662c966162778b Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 9 Aug 2016 06:05:17 +0200
Subject: [PATCH 6/6] Fix talloc hierarchy

---
 source4/dsdb/samdb/ldb_modules/resolve_oids.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/source4/dsdb/samdb/ldb_modules/resolve_oids.c b/source4/dsdb/samdb/ldb_modules/resolve_oids.c
index 5ba8475..20430c4 100644
--- a/source4/dsdb/samdb/ldb_modules/resolve_oids.c
+++ b/source4/dsdb/samdb/ldb_modules/resolve_oids.c
@@ -488,7 +488,7 @@ static const char **str_list_copy_const_clean_suffix(TALLOC_CTX *mem_ctx,
 	}
 
 	for (i=0;list && list[i];i++) {
-		ret[i] = strip_suffix(mem_ctx, list[i], ";binary");
+		ret[i] = strip_suffix(list, list[i], ";binary");
 		if (ret[i] == NULL) {
 			TALLOC_FREE(ret);
 			return NULL;
-- 
2.1.4



More information about the samba-technical mailing list