[PATCH] Some simplifications around dbwrap

Jeremy Allison jra at samba.org
Tue Aug 15 22:27:07 UTC 2017


On Fri, Aug 11, 2017 at 07:51:42AM +0200, Volker Lendecke via samba-technical wrote:
> Hi!
> 
> Review appreciated!

Really nice cleanups Volker, I love the way everything uses
a common function now.

RB+ and pushed.

> -- 
> 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 004459e8bcdf4245f02a2f7734461234d63087ce Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Wed, 9 Nov 2016 16:34:28 +0100
> Subject: [PATCH 1/8] dbwrap: Convert dbwrap_store to dbwrap_do_locked
> 
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
>  lib/dbwrap/dbwrap.c | 26 +++++++++++++++++---------
>  1 file changed, 17 insertions(+), 9 deletions(-)
> 
> diff --git a/lib/dbwrap/dbwrap.c b/lib/dbwrap/dbwrap.c
> index 22437f62083..0e576b30f60 100644
> --- a/lib/dbwrap/dbwrap.c
> +++ b/lib/dbwrap/dbwrap.c
> @@ -301,22 +301,30 @@ bool dbwrap_exists(struct db_context *db, TDB_DATA key)
>  	return (result == 1);
>  }
>  
> +struct dbwrap_store_state {
> +	TDB_DATA data;
> +	int flags;
> +	NTSTATUS status;
> +};
> +
> +static void dbwrap_store_fn(struct db_record *rec, void *private_data)
> +{
> +	struct dbwrap_store_state *state = private_data;
> +	state->status = dbwrap_record_store(rec, state->data, state->flags);
> +}
> +
>  NTSTATUS dbwrap_store(struct db_context *db, TDB_DATA key,
>  		      TDB_DATA data, int flags)
>  {
> -	struct db_record *rec;
> +	struct dbwrap_store_state state = { .data = data, .flags = flags };
>  	NTSTATUS status;
> -	TALLOC_CTX *frame = talloc_stackframe();
>  
> -	rec = dbwrap_fetch_locked(db, frame, key);
> -	if (rec == NULL) {
> -		TALLOC_FREE(frame);
> -		return NT_STATUS_NO_MEMORY;
> +	status = dbwrap_do_locked(db, key, dbwrap_store_fn, &state);
> +	if (!NT_STATUS_IS_OK(status)) {
> +		return status;
>  	}
>  
> -	status = dbwrap_record_store(rec, data, flags);
> -	TALLOC_FREE(frame);
> -	return status;
> +	return state.status;
>  }
>  
>  NTSTATUS dbwrap_delete(struct db_context *db, TDB_DATA key)
> -- 
> 2.11.0
> 
> 
> From 3f81ab925007ea52f921e1b3f3981e7019caec0a Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Wed, 9 Nov 2016 16:37:49 +0100
> Subject: [PATCH 2/8] dbwrap: Convert dbwrap_delete to dbwrap_do_locked
> 
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
>  lib/dbwrap/dbwrap.c | 25 ++++++++++++++++---------
>  1 file changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/lib/dbwrap/dbwrap.c b/lib/dbwrap/dbwrap.c
> index 0e576b30f60..e1d792fa2f8 100644
> --- a/lib/dbwrap/dbwrap.c
> +++ b/lib/dbwrap/dbwrap.c
> @@ -327,20 +327,27 @@ NTSTATUS dbwrap_store(struct db_context *db, TDB_DATA key,
>  	return state.status;
>  }
>  
> +struct dbwrap_delete_state {
> +	NTSTATUS status;
> +};
> +
> +static void dbwrap_delete_fn(struct db_record *rec, void *private_data)
> +{
> +	struct dbwrap_delete_state *state = private_data;
> +	state->status = dbwrap_record_delete(rec);
> +}
> +
>  NTSTATUS dbwrap_delete(struct db_context *db, TDB_DATA key)
>  {
> -	struct db_record *rec;
> +	struct dbwrap_delete_state state;
>  	NTSTATUS status;
> -	TALLOC_CTX *frame = talloc_stackframe();
>  
> -	rec = dbwrap_fetch_locked(db, frame, key);
> -	if (rec == NULL) {
> -		TALLOC_FREE(frame);
> -		return NT_STATUS_NO_MEMORY;
> +	status = dbwrap_do_locked(db, key, dbwrap_delete_fn, &state);
> +	if (!NT_STATUS_IS_OK(status)) {
> +		return status;
>  	}
> -	status = dbwrap_record_delete(rec);
> -	TALLOC_FREE(frame);
> -	return status;
> +
> +	return state.status;
>  }
>  
>  NTSTATUS dbwrap_traverse(struct db_context *db,
> -- 
> 2.11.0
> 
> 
> From bbb3d824057c45d07d35dadee2b8dccca4b7f6c1 Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Wed, 26 Jul 2017 14:56:53 +0200
> Subject: [PATCH 3/8] dbwrap: Simplify dbwrap_store_int32_bystring
> 
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
>  lib/dbwrap/dbwrap_util.c | 19 +++++--------------
>  1 file changed, 5 insertions(+), 14 deletions(-)
> 
> diff --git a/lib/dbwrap/dbwrap_util.c b/lib/dbwrap/dbwrap_util.c
> index 22f910de992..443a915df2d 100644
> --- a/lib/dbwrap/dbwrap_util.c
> +++ b/lib/dbwrap/dbwrap_util.c
> @@ -77,23 +77,14 @@ NTSTATUS dbwrap_fetch_int32_bystring(struct db_context *db, const char *keystr,
>  NTSTATUS dbwrap_store_int32_bystring(struct db_context *db, const char *keystr,
>  				     int32_t v)
>  {
> -	struct db_record *rec;
> -	int32_t v_store;
> +	uint8_t v_store[sizeof(int32_t)];
> +	TDB_DATA data = { .dptr = v_store, .dsize = sizeof(v_store) };
>  	NTSTATUS status;
>  
> -	rec = dbwrap_fetch_locked(db, talloc_tos(),
> -				  string_term_tdb_data(keystr));
> -	if (rec == NULL) {
> -		return NT_STATUS_UNSUCCESSFUL;
> -	}
> -
> -	SIVAL(&v_store, 0, v);
> +	SIVAL(v_store, 0, v);
>  
> -	status = dbwrap_record_store(rec,
> -				     make_tdb_data((const uint8_t *)&v_store,
> -						   sizeof(v_store)),
> -				     TDB_REPLACE);
> -	TALLOC_FREE(rec);
> +	status = dbwrap_store(db, string_term_tdb_data(keystr), data,
> +			      TDB_REPLACE);
>  	return status;
>  }
>  
> -- 
> 2.11.0
> 
> 
> From b8b41de95b73f899aa53290e3035c94ba8c5e2c4 Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Wed, 26 Jul 2017 14:56:53 +0200
> Subject: [PATCH 4/8] dbwrap: Simplify dbwrap_store_uint32_bystring
> 
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
>  lib/dbwrap/dbwrap_util.c | 19 +++++--------------
>  1 file changed, 5 insertions(+), 14 deletions(-)
> 
> diff --git a/lib/dbwrap/dbwrap_util.c b/lib/dbwrap/dbwrap_util.c
> index 443a915df2d..c7971a95b85 100644
> --- a/lib/dbwrap/dbwrap_util.c
> +++ b/lib/dbwrap/dbwrap_util.c
> @@ -133,23 +133,14 @@ NTSTATUS dbwrap_fetch_uint32_bystring(struct db_context *db,
>  NTSTATUS dbwrap_store_uint32_bystring(struct db_context *db,
>  				      const char *keystr, uint32_t v)
>  {
> -	struct db_record *rec;
> -	uint32_t v_store;
> +	uint8_t v_store[sizeof(uint32_t)];
> +	TDB_DATA data = { .dptr = v_store, .dsize = sizeof(v_store) };
>  	NTSTATUS status;
>  
> -	rec = dbwrap_fetch_locked(db, talloc_tos(),
> -				  string_term_tdb_data(keystr));
> -	if (rec == NULL) {
> -		return NT_STATUS_INVALID_PARAMETER;
> -	}
> -
> -	SIVAL(&v_store, 0, v);
> +	SIVAL(v_store, 0, v);
>  
> -	status = dbwrap_record_store(rec,
> -				     make_tdb_data((const uint8_t *)&v_store,
> -						   sizeof(v_store)),
> -				     TDB_REPLACE);
> -	TALLOC_FREE(rec);
> +	status = dbwrap_store(db, string_term_tdb_data(keystr), data,
> +			      TDB_REPLACE);
>  	return status;
>  }
>  
> -- 
> 2.11.0
> 
> 
> From b78163e8e573ce251cb0e94f30dc1dd3a6a2eaf5 Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Wed, 26 Jul 2017 15:10:55 +0200
> Subject: [PATCH 5/8] dbwrap: Simplify dbwrap_trans_store
> 
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
>  lib/dbwrap/dbwrap_util.c | 11 ++---------
>  1 file changed, 2 insertions(+), 9 deletions(-)
> 
> diff --git a/lib/dbwrap/dbwrap_util.c b/lib/dbwrap/dbwrap_util.c
> index c7971a95b85..acd3c1837bc 100644
> --- a/lib/dbwrap/dbwrap_util.c
> +++ b/lib/dbwrap/dbwrap_util.c
> @@ -344,24 +344,17 @@ struct dbwrap_store_context {
>  
>  static NTSTATUS dbwrap_store_action(struct db_context *db, void *private_data)
>  {
> -	struct db_record *rec = NULL;
>  	NTSTATUS status;
>  	struct dbwrap_store_context *store_ctx;
>  
>  	store_ctx = (struct dbwrap_store_context *)private_data;
>  
> -	rec = dbwrap_fetch_locked(db, talloc_tos(), *(store_ctx->key));
> -	if (rec == NULL) {
> -		DEBUG(5, ("fetch_locked failed\n"));
> -		return NT_STATUS_NO_MEMORY;
> -	}
> -
> -	status = dbwrap_record_store(rec, *(store_ctx->dbuf), store_ctx->flag);
> +	status = dbwrap_store(db, *(store_ctx->key), *(store_ctx->dbuf),
> +			      store_ctx->flag);
>  	if (!NT_STATUS_IS_OK(status)) {
>  		DEBUG(5, ("store returned %s\n", nt_errstr(status)));
>  	}
>  
> -	TALLOC_FREE(rec);
>  	return status;
>  }
>  
> -- 
> 2.11.0
> 
> 
> From 70aff72e6dfb4fb5de52dbb320382fc761673677 Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Wed, 26 Jul 2017 15:12:21 +0200
> Subject: [PATCH 6/8] dbwrap: Simplify dbwrap_trans_delete
> 
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
>  lib/dbwrap/dbwrap_util.c | 11 +----------
>  1 file changed, 1 insertion(+), 10 deletions(-)
> 
> diff --git a/lib/dbwrap/dbwrap_util.c b/lib/dbwrap/dbwrap_util.c
> index acd3c1837bc..cb98d742186 100644
> --- a/lib/dbwrap/dbwrap_util.c
> +++ b/lib/dbwrap/dbwrap_util.c
> @@ -376,22 +376,13 @@ NTSTATUS dbwrap_trans_store(struct db_context *db, TDB_DATA key, TDB_DATA dbuf,
>  static NTSTATUS dbwrap_delete_action(struct db_context * db, void *private_data)
>  {
>  	NTSTATUS status;
> -	struct db_record *rec;
>  	TDB_DATA *key = (TDB_DATA *)private_data;
>  
> -	rec = dbwrap_fetch_locked(db, talloc_tos(), *key);
> -	if (rec == NULL) {
> -		DEBUG(5, ("fetch_locked failed\n"));
> -		return NT_STATUS_NO_MEMORY;
> -	}
> -
> -	status = dbwrap_record_delete(rec);
> +	status = dbwrap_delete(db, *key);
>  	if (!NT_STATUS_IS_OK(status)) {
>  		DBG_INFO("dbwrap_record_delete returned %s\n",
>  			 nt_errstr(status));
>  	}
> -
> -	talloc_free(rec);
>  	return  status;
>  }
>  
> -- 
> 2.11.0
> 
> 
> From 9a57995300def955613bb32725dcadb4e8a9d0f2 Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Wed, 26 Jul 2017 15:14:51 +0200
> Subject: [PATCH 7/8] dbwrap: Simplify dbwrap_unmarshall_fn
> 
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
>  lib/dbwrap/dbwrap_util.c | 16 +++-------------
>  1 file changed, 3 insertions(+), 13 deletions(-)
> 
> diff --git a/lib/dbwrap/dbwrap_util.c b/lib/dbwrap/dbwrap_util.c
> index cb98d742186..df6dea40097 100644
> --- a/lib/dbwrap/dbwrap_util.c
> +++ b/lib/dbwrap/dbwrap_util.c
> @@ -709,22 +709,12 @@ static bool dbwrap_unmarshall_fn(TDB_DATA key, TDB_DATA value,
>  				 void *private_data)
>  {
>  	struct dbwrap_unmarshall_state *state = private_data;
> -	struct db_record *rec;
>  	NTSTATUS status;
>  
> -	rec = dbwrap_fetch_locked(state->db, state->db, key);
> -	if (rec == NULL) {
> -		DEBUG(10, ("%s: dbwrap_fetch_locked failed\n",
> -			   __func__));
> -		state->ret = NT_STATUS_NO_MEMORY;
> -		return false;
> -	}
> -
> -	status = dbwrap_record_store(rec, value, 0);
> -	TALLOC_FREE(rec);
> +	status = dbwrap_store(state->db, key, value, 0);
>  	if (!NT_STATUS_IS_OK(status)) {
> -		DEBUG(10, ("%s: dbwrap_record_store failed: %s\n",
> -			   __func__, nt_errstr(status)));
> +		DBG_DEBUG("dbwrap_record_store failed: %s\n",
> +			  nt_errstr(status));
>  		state->ret = status;
>  		return false;
>  	}
> -- 
> 2.11.0
> 
> 
> From 25eb5ca8d8353e88037ec8c12d0ec8932384b051 Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Wed, 26 Jul 2017 15:18:16 +0200
> Subject: [PATCH 8/8] libhttp: Remove an unneeded include
> 
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
>  source4/lib/http/http.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/source4/lib/http/http.c b/source4/lib/http/http.c
> index 8fac33f2c1c..659cba0ff14 100644
> --- a/source4/lib/http/http.c
> +++ b/source4/lib/http/http.c
> @@ -20,7 +20,6 @@
>  */
>  
>  #include "includes.h"
> -#include <talloc_dict.h>
>  #include "lib/util/tevent_ntstatus.h"
>  #include "http.h"
>  #include "http_internal.h"
> -- 
> 2.11.0
> 




More information about the samba-technical mailing list