[PATCH] Fix bug 13629 plus a few small cleanups

Volker Lendecke Volker.Lendecke at SerNet.DE
Sun Oct 7 18:30:47 UTC 2018


Hi!

Review appreciated!

Thanks, Volker

-- 
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:kontakt at sernet.de
-------------- next part --------------
From f50a1e4743776fe17b25deecc7ee23aec0d201da Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sun, 7 Oct 2018 14:47:26 +0200
Subject: [PATCH 1/4] lib: Avoid the use of open_memstream in
 tevent_req_profile_string

Solaris does not have it.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13629
Signed-off-by: Volker Lendecke <vl at samba.org>
---
 lib/tevent/test_req.c         | 15 +++++++-
 lib/util/tevent_req_profile.c | 85 ++++++++++++++++++-------------------------
 lib/util/tevent_req_profile.h |  8 +---
 3 files changed, 51 insertions(+), 57 deletions(-)

diff --git a/lib/tevent/test_req.c b/lib/tevent/test_req.c
index 565ef31024f..2274a8cadc8 100644
--- a/lib/tevent/test_req.c
+++ b/lib/tevent/test_req.c
@@ -170,6 +170,7 @@ static bool test_tevent_req_profile2(struct torture_context *tctx,
 	pid_t pid1, pid2;
 	enum tevent_req_state state1, state2;
 	uint64_t err1, err2;
+	char *printstring;
 	ssize_t pack_len;
 	int err;
 	bool ok;
@@ -189,7 +190,12 @@ static bool test_tevent_req_profile2(struct torture_context *tctx,
 	TALLOC_FREE(req);
 	TALLOC_FREE(ev);
 
-	tevent_req_profile_print(p1, stdout, 0, UINT_MAX);
+	printstring = tevent_req_profile_string(tctx, p1, 0, UINT_MAX);
+	torture_assert_not_null(
+		tctx,
+		printstring,
+		"tevent_req_profile_string failed\n");
+	printf("%s\n", printstring);
 
 	pack_len = tevent_req_profile_pack(p1, NULL, 0);
 	torture_assert(tctx, pack_len>0, "profile_pack failed\n");
@@ -212,7 +218,12 @@ static bool test_tevent_req_profile2(struct torture_context *tctx,
 					 "profile_unpack failed\n");
 	}
 
-	tevent_req_profile_print(p2, stdout, 0, UINT_MAX);
+	printstring = tevent_req_profile_string(tctx, p2, 0, UINT_MAX);
+	torture_assert_not_null(
+		tctx,
+		printstring,
+		"tevent_req_profile_string failed\n");
+	printf("%s\n", printstring);
 
 	tevent_req_profile_get_name(p1, &str1);
 	tevent_req_profile_get_name(p2, &str2);
diff --git a/lib/util/tevent_req_profile.c b/lib/util/tevent_req_profile.c
index 522741c5ede..2d280f78f32 100644
--- a/lib/util/tevent_req_profile.c
+++ b/lib/util/tevent_req_profile.c
@@ -29,10 +29,11 @@
 #include "lib/util/time_basic.h"
 #include "lib/util/memory.h"
 
-int tevent_req_profile_print(const struct tevent_req_profile *profile,
-			     FILE *fp,
-			     unsigned indent,
-			     unsigned max_indent)
+static bool tevent_req_profile_string_internal(
+	const struct tevent_req_profile *profile,
+	unsigned indent,
+	unsigned max_indent,
+	char **string)
 {
 	struct timeval start, stop, diff;
 	struct timeval_buf start_buf, stop_buf;
@@ -44,7 +45,7 @@ int tevent_req_profile_print(const struct tevent_req_profile *profile,
 	const char *state_buf = NULL;
 	uint64_t user_error;
 	const struct tevent_req_profile *sub = NULL;
-	int ret;
+	char *result;
 
 	tevent_req_profile_get_name(profile, &req_name);
 
@@ -85,8 +86,8 @@ int tevent_req_profile_print(const struct tevent_req_profile *profile,
 		break;
 	}
 
-	ret = fprintf(
-		fp,
+	result = talloc_asprintf_append_buffer(
+		*string,
 		"%*s[%s] %s [%s] %s [%s] [%ju.%.6ju] -> %s (%d %"PRIu64"))\n",
 		indent,
 		"",
@@ -100,72 +101,58 @@ int tevent_req_profile_print(const struct tevent_req_profile *profile,
 		state_buf,
 		(int)state,
 		user_error);
-
-	if (ret < 0) {
-		return ret;
+	if (result == NULL) {
+		return false;
 	}
+	*string = result;
 
 	indent += 1;
 
 	if (indent >= max_indent) {
-		return ret;
+		return true;
 	}
 
 	for (sub = tevent_req_profile_get_subprofiles(profile);
 	     sub != NULL;
 	     sub = tevent_req_profile_next(sub)) {
-		int subret;
-
-		subret = tevent_req_profile_print(sub, fp, indent, max_indent);
-		if (subret < 0) {
-			return subret;
-		}
-
-		ret += subret;
-
-		if (ret < subret) {
-			/* overflow */
-			return -1;
+		bool ret;
+
+		ret = tevent_req_profile_string_internal(
+			sub,
+			indent,
+			max_indent,
+			string);
+		if (!ret) {
+			return false;
 		}
 	}
 
-	return ret;
+	return true;
 }
 
-char *tevent_req_profile_string(const struct tevent_req_profile *profile,
-				TALLOC_CTX *mem_ctx,
+char *tevent_req_profile_string(TALLOC_CTX *mem_ctx,
+				const struct tevent_req_profile *profile,
 				unsigned indent,
 				unsigned max_indent)
 {
-	FILE *fp = NULL;
-	char *buf = NULL;
-	size_t buflen = 0;
-	char *result = NULL;
-	int ret;
+	char *result;
+	bool ret;
 
-	fp = open_memstream(&buf, &buflen);
-	if (fp == NULL) {
+	result = talloc_strdup(mem_ctx, "");
+	if (result == NULL) {
 		return NULL;
 	}
 
-	ret = tevent_req_profile_print(profile, fp, 0, max_indent);
-	if (ret < 0) {
-		goto done;
-	}
-
-	ret = fclose(fp);
-	if (ret != 0) {
-		goto done;
+	ret = tevent_req_profile_string_internal(
+		profile,
+		indent,
+		max_indent,
+		&result);
+	if (!ret) {
+		TALLOC_FREE(result);
+		return NULL;
 	}
 
-	/*
-	 * A FILE* from open_memstream maintains the 0-byte at the end
-	 * beyond the reported length.
-	 */
-	result = talloc_memdup(mem_ctx, buf, buflen+1);
-
-done:
-	SAFE_FREE(buf);
 	return result;
 }
 
diff --git a/lib/util/tevent_req_profile.h b/lib/util/tevent_req_profile.h
index 00dbc5a9cc2..3bcdbc1a289 100644
--- a/lib/util/tevent_req_profile.h
+++ b/lib/util/tevent_req_profile.h
@@ -29,12 +29,8 @@
 #include "replace.h"
 #include <tevent.h>
 
-int tevent_req_profile_print(const struct tevent_req_profile *profile,
-			     FILE *fp,
-			     unsigned indent,
-			     unsigned max_indent);
-char *tevent_req_profile_string(const struct tevent_req_profile *profile,
-				TALLOC_CTX *mem_ctx,
+char *tevent_req_profile_string(TALLOC_CTX *mem_ctx,
+				const struct tevent_req_profile *profile,
 				unsigned indent,
 				unsigned max_indent);
 ssize_t tevent_req_profile_pack(
-- 
2.11.0


From bd8d33097871fb512267053359d4bf9b2e36b346 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Fri, 5 Oct 2018 12:12:39 +0200
Subject: [PATCH 2/4] pdb: Use "sid_compose" where appropriate

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/auth/token_util.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/source3/auth/token_util.c b/source3/auth/token_util.c
index f5b0e694433..c95d54db671 100644
--- a/source3/auth/token_util.c
+++ b/source3/auth/token_util.c
@@ -220,8 +220,7 @@ static NTSTATUS add_builtin_guests(struct security_token *token,
 	/*
 	 * First check the local GUEST account.
 	 */
-	sid_copy(&tmp_sid, get_global_sam_sid());
-	sid_append_rid(&tmp_sid, DOMAIN_RID_GUEST);
+	sid_compose(&tmp_sid, get_global_sam_sid(), DOMAIN_RID_GUEST);
 
 	if (nt_token_check_sid(&tmp_sid, token)) {
 		status = add_sid_to_array_unique(token,
@@ -237,8 +236,7 @@ static NTSTATUS add_builtin_guests(struct security_token *token,
 	/*
 	 * First check the local GUESTS group.
 	 */
-	sid_copy(&tmp_sid, get_global_sam_sid());
-	sid_append_rid(&tmp_sid, DOMAIN_RID_GUESTS);
+	sid_compose(&tmp_sid, get_global_sam_sid(), DOMAIN_RID_GUESTS);
 
 	if (nt_token_check_sid(&tmp_sid, token)) {
 		status = add_sid_to_array_unique(token,
-- 
2.11.0


From f4c2b8336d2686050f9f1ccb614dcd667e6dd009 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Fri, 5 Oct 2018 11:34:41 +0200
Subject: [PATCH 3/4] pdb: Fix some "(ret == true)" to just "(ret)"

"ret" is a boolean, so this should not change semantics

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/passdb/pdb_interface.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/source3/passdb/pdb_interface.c b/source3/passdb/pdb_interface.c
index 8b23da6b979..315da52f01b 100644
--- a/source3/passdb/pdb_interface.c
+++ b/source3/passdb/pdb_interface.c
@@ -1238,7 +1238,7 @@ bool pdb_sid_to_id(const struct dom_sid *sid, struct unixid *id)
 
 	ret = pdb->sid_to_id(pdb, sid, id);
 
-	if (ret == true) {
+	if (ret) {
 		idmap_cache_set_sid2unixid(sid, id);
 	}
 
@@ -1540,7 +1540,7 @@ static bool pdb_default_sid_to_id(struct pdb_methods *methods,
 	 * "Unix User" and "Unix Group"
 	 */
 	ret = pdb_sid_to_id_unix_users_and_groups(sid, id);
-	if (ret == true) {
+	if (ret) {
 		goto done;
 	}
 
-- 
2.11.0


From 518060a12547b5c84b99d76470e93aff44262860 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Fri, 5 Oct 2018 14:49:17 +0200
Subject: [PATCH 4/4] pdb: Reduce code duplication in make_user_info()

10 lines less and a few hundred (-O0) bytes .text less

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/auth/user_info.c | 42 ++++++++++++++++--------------------------
 1 file changed, 16 insertions(+), 26 deletions(-)

diff --git a/source3/auth/user_info.c b/source3/auth/user_info.c
index c79cc0c4f35..81192a51c61 100644
--- a/source3/auth/user_info.c
+++ b/source3/auth/user_info.c
@@ -73,53 +73,45 @@ NTSTATUS make_user_info(TALLOC_CTX *mem_ctx,
 
 	user_info->client.account_name = talloc_strdup(user_info, smb_name);
 	if (user_info->client.account_name == NULL) {
-		TALLOC_FREE(user_info);
-		return NT_STATUS_NO_MEMORY;
+		goto nomem;
 	}
 
 	user_info->mapped.account_name = talloc_strdup(user_info, internal_username);
 	if (user_info->mapped.account_name == NULL) {
-		TALLOC_FREE(user_info);
-		return NT_STATUS_NO_MEMORY;
+		goto nomem;
 	}
 
 	user_info->mapped.domain_name = talloc_strdup(user_info, domain);
 	if (user_info->mapped.domain_name == NULL) {
-		TALLOC_FREE(user_info);
-		return NT_STATUS_NO_MEMORY;
+		goto nomem;
 	}
 
 	user_info->client.domain_name = talloc_strdup(user_info, client_domain);
 	if (user_info->client.domain_name == NULL) {
-		TALLOC_FREE(user_info);
-		return NT_STATUS_NO_MEMORY;
+		goto nomem;
 	}
 
 	user_info->workstation_name = talloc_strdup(user_info, workstation_name);
 	if (user_info->workstation_name == NULL) {
-		TALLOC_FREE(user_info);
-		return NT_STATUS_NO_MEMORY;
+		goto nomem;
 	}
 
 	user_info->remote_host = tsocket_address_copy(remote_address, user_info);
 	if (user_info->remote_host == NULL) {
-		TALLOC_FREE(user_info);
-		return NT_STATUS_NO_MEMORY;
+		goto nomem;
 	}
 
 	if (local_address != NULL) {
 		user_info->local_host = tsocket_address_copy(local_address,
 							     user_info);
 		if (user_info->local_host == NULL) {
-			TALLOC_FREE(user_info);
-			return NT_STATUS_NO_MEMORY;
+			goto nomem;
 		}
 	}
 
 	user_info->service_description = talloc_strdup(user_info, service_description);
 	if (user_info->service_description == NULL) {
-		TALLOC_FREE(user_info);
-		return NT_STATUS_NO_MEMORY;
+		goto nomem;
 	}
 
 	DEBUG(5,("making blobs for %s's user_info struct\n", internal_username));
@@ -127,22 +119,19 @@ NTSTATUS make_user_info(TALLOC_CTX *mem_ctx,
 	if (lm_pwd && lm_pwd->data) {
 		user_info->password.response.lanman = data_blob_talloc(user_info, lm_pwd->data, lm_pwd->length);
 		if (user_info->password.response.lanman.data == NULL) {
-			TALLOC_FREE(user_info);
-			return NT_STATUS_NO_MEMORY;
+			goto nomem;
 		}
 	}
 	if (nt_pwd && nt_pwd->data) {
 		user_info->password.response.nt = data_blob_talloc(user_info, nt_pwd->data, nt_pwd->length);
 		if (user_info->password.response.nt.data == NULL) {
-			TALLOC_FREE(user_info);
-			return NT_STATUS_NO_MEMORY;
+			goto nomem;
 		}
 	}
 	if (lm_interactive_pwd) {
 		user_info->password.hash.lanman = talloc(user_info, struct samr_Password);
 		if (user_info->password.hash.lanman == NULL) {
-			TALLOC_FREE(user_info);
-			return NT_STATUS_NO_MEMORY;
+			goto nomem;
 		}
 		memcpy(user_info->password.hash.lanman->hash, lm_interactive_pwd->hash,
 		       sizeof(user_info->password.hash.lanman->hash));
@@ -152,8 +141,7 @@ NTSTATUS make_user_info(TALLOC_CTX *mem_ctx,
 	if (nt_interactive_pwd) {
 		user_info->password.hash.nt = talloc(user_info, struct samr_Password);
 		if (user_info->password.hash.nt == NULL) {
-			TALLOC_FREE(user_info);
-			return NT_STATUS_NO_MEMORY;
+			goto nomem;
 		}
 		memcpy(user_info->password.hash.nt->hash, nt_interactive_pwd->hash,
 		       sizeof(user_info->password.hash.nt->hash));
@@ -163,8 +151,7 @@ NTSTATUS make_user_info(TALLOC_CTX *mem_ctx,
 	if (plaintext_password) {
 		user_info->password.plaintext = talloc_strdup(user_info, plaintext_password);
 		if (user_info->password.plaintext == NULL) {
-			TALLOC_FREE(user_info);
-			return NT_STATUS_NO_MEMORY;
+			goto nomem;
 		}
 		talloc_set_destructor(user_info->password.plaintext, clear_string);
 	}
@@ -176,4 +163,7 @@ NTSTATUS make_user_info(TALLOC_CTX *mem_ctx,
 	DEBUG(10,("made a user_info for %s (%s)\n", internal_username, smb_name));
 	*ret_user_info = user_info;
 	return NT_STATUS_OK;
+nomem:
+	TALLOC_FREE(user_info);
+	return NT_STATUS_NO_MEMORY;
 }
-- 
2.11.0



More information about the samba-technical mailing list