[PATCH] Fix bug 13629 plus a few small cleanups
Jeremy Allison
jra at samba.org
Mon Oct 8 20:17:33 UTC 2018
On Sun, Oct 07, 2018 at 08:30:47PM +0200, Volker Lendecke via samba-technical wrote:
> Hi!
>
> Review appreciated!
RB+. LGTM and pushed, thanks for helping out the Solaris/HPUX's of
this world :-).
Jeremy.
> --
> 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
> 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