[PATCHES] more smbXsrv fetch_lock refactoring

Jeremy Allison jra at samba.org
Sun Feb 28 03:57:26 UTC 2016


On Sat, Feb 27, 2016 at 02:34:46AM +0100, Michael Adam wrote:
> Attached find a few patches that do
> more refactoring of key-generation and
> fetch-lock into helper functions for
> the remaining modules smbXsrv_{tcon,session,client}
> analogous to the previous patches to smbXsrv_open.c
> 
> Review appreciated!

Nice cleanup - +1 for more functions :-).

LGTM. Pushed.

> From eda6b55aaf219abe0322f33a4698be1ac58fed58 Mon Sep 17 00:00:00 2001
> From: Michael Adam <obnox at samba.org>
> Date: Sat, 27 Feb 2016 00:52:59 +0100
> Subject: [PATCH 1/5] smbXsrv_tcon: factor fetch-locking of global record into
>  function
> 
> Signed-off-by: Michael Adam <obnox at samba.org>
> ---
>  source3/smbd/smbXsrv_tcon.c | 61 ++++++++++++++++++++++-----------------------
>  1 file changed, 30 insertions(+), 31 deletions(-)
> 
> diff --git a/source3/smbd/smbXsrv_tcon.c b/source3/smbd/smbXsrv_tcon.c
> index 1d2a141..5f10c49 100644
> --- a/source3/smbd/smbXsrv_tcon.c
> +++ b/source3/smbd/smbXsrv_tcon.c
> @@ -150,6 +150,27 @@ static NTSTATUS smbXsrv_tcon_local_key_to_id(TDB_DATA key, uint32_t *id)
>  	return NT_STATUS_OK;
>  }
>  
> +static struct db_record *smbXsrv_tcon_global_fetch_locked(
> +			struct db_context *db,
> +			uint32_t id,
> +			TALLOC_CTX *mem_ctx)
> +{
> +	TDB_DATA key;
> +	uint8_t key_buf[SMBXSRV_TCON_GLOBAL_TDB_KEY_SIZE];
> +	struct db_record *rec = NULL;
> +
> +	key = smbXsrv_tcon_global_id_to_key(id, key_buf);
> +
> +	rec = dbwrap_fetch_locked(db, mem_ctx, key);
> +
> +	if (rec == NULL) {
> +		DBG_DEBUG("Failed to lock global id 0x%08x, key '%s'\n", id,
> +			  hex_encode_talloc(talloc_tos(), key.dptr, key.dsize));
> +	}
> +
> +	return rec;
> +}
> +
>  static NTSTATUS smbXsrv_tcon_table_init(TALLOC_CTX *mem_ctx,
>  					struct smbXsrv_tcon_table *table,
>  					uint32_t lowest_id,
> @@ -473,8 +494,6 @@ static NTSTATUS smbXsrv_tcon_global_allocate(struct db_context *db,
>  		bool is_free = false;
>  		bool was_free = false;
>  		uint32_t id;
> -		uint8_t key_buf[SMBXSRV_TCON_GLOBAL_TDB_KEY_SIZE];
> -		TDB_DATA key;
>  
>  		if (i >= min_tries && last_free != 0) {
>  			id = last_free;
> @@ -488,9 +507,8 @@ static NTSTATUS smbXsrv_tcon_global_allocate(struct db_context *db,
>  			id--;
>  		}
>  
> -		key = smbXsrv_tcon_global_id_to_key(id, key_buf);
> -
> -		global->db_rec = dbwrap_fetch_locked(db, mem_ctx, key);
> +		global->db_rec = smbXsrv_tcon_global_fetch_locked(db, id,
> +								  mem_ctx);
>  		if (global->db_rec == NULL) {
>  			talloc_free(global);
>  			return NT_STATUS_INSUFFICIENT_RESOURCES;
> @@ -820,8 +838,6 @@ NTSTATUS smbXsrv_tcon_update(struct smbXsrv_tcon *tcon)
>  {
>  	struct smbXsrv_tcon_table *table = tcon->table;
>  	NTSTATUS status;
> -	uint8_t key_buf[SMBXSRV_TCON_GLOBAL_TDB_KEY_SIZE];
> -	TDB_DATA key;
>  
>  	if (tcon->global->db_rec != NULL) {
>  		DEBUG(0, ("smbXsrv_tcon_update(0x%08x): "
> @@ -830,17 +846,11 @@ NTSTATUS smbXsrv_tcon_update(struct smbXsrv_tcon *tcon)
>  		return NT_STATUS_INTERNAL_ERROR;
>  	}
>  
> -	key = smbXsrv_tcon_global_id_to_key(tcon->global->tcon_global_id,
> -					    key_buf);
> -
> -	tcon->global->db_rec = dbwrap_fetch_locked(table->global.db_ctx,
> -						   tcon->global, key);
> +	tcon->global->db_rec = smbXsrv_tcon_global_fetch_locked(
> +						table->global.db_ctx,
> +						tcon->global->tcon_global_id,
> +						tcon->global /* TALLOC_CTX */);
>  	if (tcon->global->db_rec == NULL) {
> -		DEBUG(0, ("smbXsrv_tcon_update(0x%08x): "
> -			  "Failed to lock global key '%s'\n",
> -			  tcon->global->tcon_global_id,
> -			  hex_encode_talloc(talloc_tos(), key.dptr,
> -					    key.dsize)));
>  		return NT_STATUS_INTERNAL_DB_ERROR;
>  	}
>  
> @@ -888,22 +898,11 @@ NTSTATUS smbXsrv_tcon_disconnect(struct smbXsrv_tcon *tcon, uint64_t vuid)
>  	global_rec = tcon->global->db_rec;
>  	tcon->global->db_rec = NULL;
>  	if (global_rec == NULL) {
> -		uint8_t key_buf[SMBXSRV_TCON_GLOBAL_TDB_KEY_SIZE];
> -		TDB_DATA key;
> -
> -		key = smbXsrv_tcon_global_id_to_key(
> +		global_rec = smbXsrv_tcon_global_fetch_locked(
> +						table->global.db_ctx,
>  						tcon->global->tcon_global_id,
> -						key_buf);
> -
> -		global_rec = dbwrap_fetch_locked(table->global.db_ctx,
> -						 tcon->global, key);
> +						tcon->global /* TALLOC_CTX */);
>  		if (global_rec == NULL) {
> -			DEBUG(0, ("smbXsrv_tcon_disconnect(0x%08x, '%s'): "
> -				  "Failed to lock global key '%s'\n",
> -				  tcon->global->tcon_global_id,
> -				  tcon->global->share_name,
> -				  hex_encode_talloc(global_rec, key.dptr,
> -						    key.dsize)));
>  			error = NT_STATUS_INTERNAL_ERROR;
>  		}
>  	}
> -- 
> 2.5.0
> 
> 
> From 8fdc0b6bbe8f361c944c1c6f036433f31a81e2f3 Mon Sep 17 00:00:00 2001
> From: Michael Adam <obnox at samba.org>
> Date: Sat, 27 Feb 2016 01:06:13 +0100
> Subject: [PATCH 2/5] smbXsrv_tcon: factor fetch-locking of local record into
>  function
> 
> Signed-off-by: Michael Adam <obnox at samba.org>
> ---
>  source3/smbd/smbXsrv_tcon.c | 58 ++++++++++++++++++++++-----------------------
>  1 file changed, 29 insertions(+), 29 deletions(-)
> 
> diff --git a/source3/smbd/smbXsrv_tcon.c b/source3/smbd/smbXsrv_tcon.c
> index 5f10c49..ddd03f6 100644
> --- a/source3/smbd/smbXsrv_tcon.c
> +++ b/source3/smbd/smbXsrv_tcon.c
> @@ -171,6 +171,27 @@ static struct db_record *smbXsrv_tcon_global_fetch_locked(
>  	return rec;
>  }
>  
> +static struct db_record *smbXsrv_tcon_local_fetch_locked(
> +			struct db_context *db,
> +			uint32_t id,
> +			TALLOC_CTX *mem_ctx)
> +{
> +	TDB_DATA key;
> +	uint8_t key_buf[SMBXSRV_TCON_LOCAL_TDB_KEY_SIZE];
> +	struct db_record *rec = NULL;
> +
> +	key = smbXsrv_tcon_local_id_to_key(id, key_buf);
> +
> +	rec = dbwrap_fetch_locked(db, mem_ctx, key);
> +
> +	if (rec == NULL) {
> +		DBG_DEBUG("Failed to lock local id 0x%08x, key '%s'\n", id,
> +			  hex_encode_talloc(talloc_tos(), key.dptr, key.dsize));
> +	}
> +
> +	return rec;
> +}
> +
>  static NTSTATUS smbXsrv_tcon_table_init(TALLOC_CTX *mem_ctx,
>  					struct smbXsrv_tcon_table *table,
>  					uint32_t lowest_id,
> @@ -287,8 +308,6 @@ static NTSTATUS smb1srv_tcon_local_allocate_id(struct db_context *db,
>  
>  	for (i = 0; i < (range / 2); i++) {
>  		uint32_t id;
> -		uint8_t key_buf[SMBXSRV_TCON_LOCAL_TDB_KEY_SIZE];
> -		TDB_DATA key;
>  		TDB_DATA val;
>  		struct db_record *rec = NULL;
>  
> @@ -302,9 +321,7 @@ static NTSTATUS smb1srv_tcon_local_allocate_id(struct db_context *db,
>  			id = highest_id;
>  		}
>  
> -		key = smbXsrv_tcon_local_id_to_key(id, key_buf);
> -
> -		rec = dbwrap_fetch_locked(db, mem_ctx, key);
> +		rec = smbXsrv_tcon_local_fetch_locked(db, id, mem_ctx);
>  		if (rec == NULL) {
>  			return NT_STATUS_INSUFFICIENT_RESOURCES;
>  		}
> @@ -354,16 +371,12 @@ static NTSTATUS smb1srv_tcon_local_allocate_id(struct db_context *db,
>  
>  	if (NT_STATUS_IS_OK(state.status)) {
>  		uint32_t id;
> -		uint8_t key_buf[SMBXSRV_TCON_LOCAL_TDB_KEY_SIZE];
> -		TDB_DATA key;
>  		TDB_DATA val;
>  		struct db_record *rec = NULL;
>  
>  		id = state.useable_id;
>  
> -		key = smbXsrv_tcon_local_id_to_key(id, key_buf);
> -
> -		rec = dbwrap_fetch_locked(db, mem_ctx, key);
> +		rec = smbXsrv_tcon_local_fetch_locked(db, id, mem_ctx);
>  		if (rec == NULL) {
>  			return NT_STATUS_INSUFFICIENT_RESOURCES;
>  		}
> @@ -755,17 +768,14 @@ static NTSTATUS smbXsrv_tcon_create(struct smbXsrv_tcon_table *table,
>  
>  	if (protocol >= PROTOCOL_SMB2_02) {
>  		uint64_t id = global->tcon_global_id;
> -		uint8_t key_buf[SMBXSRV_TCON_LOCAL_TDB_KEY_SIZE];
> -		TDB_DATA key;
>  
>  		global->tcon_wire_id = id;
>  
>  		tcon->local_id = global->tcon_global_id;
>  
> -		key = smbXsrv_tcon_local_id_to_key(tcon->local_id, key_buf);
> -
> -		local_rec = dbwrap_fetch_locked(table->local.db_ctx,
> -						tcon, key);
> +		local_rec = smbXsrv_tcon_local_fetch_locked(table->local.db_ctx,
> +							tcon->local_id,
> +							tcon /* TALLOC_CTX */);
>  		if (local_rec == NULL) {
>  			TALLOC_FREE(tcon);
>  			return NT_STATUS_NO_MEMORY;
> @@ -926,20 +936,10 @@ NTSTATUS smbXsrv_tcon_disconnect(struct smbXsrv_tcon *tcon, uint64_t vuid)
>  
>  	local_rec = tcon->db_rec;
>  	if (local_rec == NULL) {
> -		uint8_t key_buf[SMBXSRV_TCON_LOCAL_TDB_KEY_SIZE];
> -		TDB_DATA key;
> -
> -		key = smbXsrv_tcon_local_id_to_key(tcon->local_id, key_buf);
> -
> -		local_rec = dbwrap_fetch_locked(table->local.db_ctx,
> -						tcon, key);
> +		local_rec = smbXsrv_tcon_local_fetch_locked(table->local.db_ctx,
> +							tcon->local_id,
> +							tcon /* TALLOC_CTX */);
>  		if (local_rec == NULL) {
> -			DEBUG(0, ("smbXsrv_tcon_disconnect(0x%08x, '%s'): "
> -				  "Failed to lock local key '%s'\n",
> -				  tcon->global->tcon_global_id,
> -				  tcon->global->share_name,
> -				  hex_encode_talloc(local_rec, key.dptr,
> -						    key.dsize)));
>  			error = NT_STATUS_INTERNAL_ERROR;
>  		}
>  	}
> -- 
> 2.5.0
> 
> 
> From 13b38c4c8cfa2d913669531dfbe434ae321e4167 Mon Sep 17 00:00:00 2001
> From: Michael Adam <obnox at samba.org>
> Date: Sat, 27 Feb 2016 01:26:16 +0100
> Subject: [PATCH 3/5] smbXsrv_session: factor fetch-locking of global record
>  into function
> 
> Signed-off-by: Michael Adam <obnox at samba.org>
> ---
>  source3/smbd/smbXsrv_session.c | 69 ++++++++++++++++++++----------------------
>  1 file changed, 33 insertions(+), 36 deletions(-)
> 
> diff --git a/source3/smbd/smbXsrv_session.c b/source3/smbd/smbXsrv_session.c
> index 732388b..f7cc629 100644
> --- a/source3/smbd/smbXsrv_session.c
> +++ b/source3/smbd/smbXsrv_session.c
> @@ -161,6 +161,27 @@ static NTSTATUS smbXsrv_session_local_key_to_id(TDB_DATA key, uint32_t *id)
>  	return NT_STATUS_OK;
>  }
>  
> +static struct db_record *smbXsrv_session_global_fetch_locked(
> +			struct db_context *db,
> +			uint32_t id,
> +			TALLOC_CTX *mem_ctx)
> +{
> +	TDB_DATA key;
> +	uint8_t key_buf[SMBXSRV_SESSION_GLOBAL_TDB_KEY_SIZE];
> +	struct db_record *rec = NULL;
> +
> +	key = smbXsrv_session_global_id_to_key(id, key_buf);
> +
> +	rec = dbwrap_fetch_locked(db, mem_ctx, key);
> +
> +	if (rec == NULL) {
> +		DBG_DEBUG("Failed to lock global id 0x%08x, key '%s'\n", id,
> +			  hex_encode_talloc(talloc_tos(), key.dptr, key.dsize));
> +	}
> +
> +	return rec;
> +}
> +
>  static void smbXsrv_session_close_loop(struct tevent_req *subreq);
>  
>  static NTSTATUS smbXsrv_session_table_init(struct smbXsrv_connection *conn,
> @@ -693,8 +714,6 @@ static NTSTATUS smbXsrv_session_global_allocate(struct db_context *db,
>  		bool is_free = false;
>  		bool was_free = false;
>  		uint32_t id;
> -		uint8_t key_buf[SMBXSRV_SESSION_GLOBAL_TDB_KEY_SIZE];
> -		TDB_DATA key;
>  
>  		if (i >= min_tries && last_free != 0) {
>  			id = last_free;
> @@ -708,9 +727,8 @@ static NTSTATUS smbXsrv_session_global_allocate(struct db_context *db,
>  			id--;
>  		}
>  
> -		key = smbXsrv_session_global_id_to_key(id, key_buf);
> -
> -		global->db_rec = dbwrap_fetch_locked(db, mem_ctx, key);
> +		global->db_rec = smbXsrv_session_global_fetch_locked(db, id,
> +								     mem_ctx);
>  		if (global->db_rec == NULL) {
>  			talloc_free(global);
>  			return NT_STATUS_INSUFFICIENT_RESOURCES;
> @@ -931,8 +949,6 @@ struct tevent_req *smb2srv_session_close_previous_send(TALLOC_CTX *mem_ctx,
>  	uint64_t global_zeros = previous_session_id & 0xFFFFFFFF00000000LLU;
>  	struct smbXsrv_session_table *table = conn->client->session_table;
>  	struct security_token *current_token = NULL;
> -	uint8_t key_buf[SMBXSRV_SESSION_GLOBAL_TDB_KEY_SIZE];
> -	TDB_DATA key;
>  
>  	req = tevent_req_create(mem_ctx, &state,
>  				struct smb2srv_session_close_previous_state);
> @@ -969,10 +985,10 @@ struct tevent_req *smb2srv_session_close_previous_send(TALLOC_CTX *mem_ctx,
>  		return tevent_req_post(req, ev);
>  	}
>  
> -	key = smbXsrv_session_global_id_to_key(global_id, key_buf);
> -
> -	state->db_rec = dbwrap_fetch_locked(table->global.db_ctx,
> -					    state, key);
> +	state->db_rec = smbXsrv_session_global_fetch_locked(
> +							table->global.db_ctx,
> +							global_id,
> +							state /* TALLOC_CTX */);
>  	if (state->db_rec == NULL) {
>  		tevent_req_nterror(req, NT_STATUS_UNSUCCESSFUL);
>  		return tevent_req_post(req, ev);
> @@ -1336,8 +1352,6 @@ NTSTATUS smbXsrv_session_update(struct smbXsrv_session *session)
>  {
>  	struct smbXsrv_session_table *table = session->table;
>  	NTSTATUS status;
> -	uint8_t key_buf[SMBXSRV_SESSION_GLOBAL_TDB_KEY_SIZE];
> -	TDB_DATA key;
>  
>  	if (session->global->db_rec != NULL) {
>  		DEBUG(0, ("smbXsrv_session_update(0x%08x): "
> @@ -1346,18 +1360,11 @@ NTSTATUS smbXsrv_session_update(struct smbXsrv_session *session)
>  		return NT_STATUS_INTERNAL_ERROR;
>  	}
>  
> -	key = smbXsrv_session_global_id_to_key(
> +	session->global->db_rec = smbXsrv_session_global_fetch_locked(
> +					table->global.db_ctx,
>  					session->global->session_global_id,
> -					key_buf);
> -
> -	session->global->db_rec = dbwrap_fetch_locked(table->global.db_ctx,
> -						      session->global, key);
> +					session->global /* TALLOC_CTX */);
>  	if (session->global->db_rec == NULL) {
> -		DEBUG(0, ("smbXsrv_session_update(0x%08x): "
> -			  "Failed to lock global key '%s'\n",
> -			  session->global->session_global_id,
> -			  hex_encode_talloc(talloc_tos(), key.dptr,
> -					    key.dsize)));
>  		return NT_STATUS_INTERNAL_DB_ERROR;
>  	}
>  
> @@ -1621,21 +1628,11 @@ NTSTATUS smbXsrv_session_logoff(struct smbXsrv_session *session)
>  	global_rec = session->global->db_rec;
>  	session->global->db_rec = NULL;
>  	if (global_rec == NULL) {
> -		uint8_t key_buf[SMBXSRV_SESSION_GLOBAL_TDB_KEY_SIZE];
> -		TDB_DATA key;
> -
> -		key = smbXsrv_session_global_id_to_key(
> +		global_rec = smbXsrv_session_global_fetch_locked(
> +					table->global.db_ctx,
>  					session->global->session_global_id,
> -					key_buf);
> -
> -		global_rec = dbwrap_fetch_locked(table->global.db_ctx,
> -						 session->global, key);
> +					session->global /* TALLOC_CTX */);
>  		if (global_rec == NULL) {
> -			DEBUG(0, ("smbXsrv_session_logoff(0x%08x): "
> -				  "Failed to lock global key '%s'\n",
> -				  session->global->session_global_id,
> -				  hex_encode_talloc(global_rec, key.dptr,
> -						    key.dsize)));
>  			error = NT_STATUS_INTERNAL_ERROR;
>  		}
>  	}
> -- 
> 2.5.0
> 
> 
> From 8a26f5647833b5fb6a3c81c692445b57f0febba2 Mon Sep 17 00:00:00 2001
> From: Michael Adam <obnox at samba.org>
> Date: Sat, 27 Feb 2016 01:37:34 +0100
> Subject: [PATCH 4/5] smbXsrv_session: factor fetch-locking of local record
>  into function
> 
> Signed-off-by: Michael Adam <obnox at samba.org>
> ---
>  source3/smbd/smbXsrv_session.c | 60 ++++++++++++++++++++++--------------------
>  1 file changed, 31 insertions(+), 29 deletions(-)
> 
> diff --git a/source3/smbd/smbXsrv_session.c b/source3/smbd/smbXsrv_session.c
> index f7cc629..a5aee8c 100644
> --- a/source3/smbd/smbXsrv_session.c
> +++ b/source3/smbd/smbXsrv_session.c
> @@ -182,6 +182,27 @@ static struct db_record *smbXsrv_session_global_fetch_locked(
>  	return rec;
>  }
>  
> +static struct db_record *smbXsrv_session_local_fetch_locked(
> +			struct db_context *db,
> +			uint32_t id,
> +			TALLOC_CTX *mem_ctx)
> +{
> +	TDB_DATA key;
> +	uint8_t key_buf[SMBXSRV_SESSION_LOCAL_TDB_KEY_SIZE];
> +	struct db_record *rec = NULL;
> +
> +	key = smbXsrv_session_local_id_to_key(id, key_buf);
> +
> +	rec = dbwrap_fetch_locked(db, mem_ctx, key);
> +
> +	if (rec == NULL) {
> +		DBG_DEBUG("Failed to lock local id 0x%08x, key '%s'\n", id,
> +			  hex_encode_talloc(talloc_tos(), key.dptr, key.dsize));
> +	}
> +
> +	return rec;
> +}
> +
>  static void smbXsrv_session_close_loop(struct tevent_req *subreq);
>  
>  static NTSTATUS smbXsrv_session_table_init(struct smbXsrv_connection *conn,
> @@ -483,8 +504,6 @@ static NTSTATUS smb1srv_session_local_allocate_id(struct db_context *db,
>  
>  	for (i = 0; i < (range / 2); i++) {
>  		uint32_t id;
> -		uint8_t key_buf[SMBXSRV_SESSION_LOCAL_TDB_KEY_SIZE];
> -		TDB_DATA key;
>  		TDB_DATA val;
>  		struct db_record *rec = NULL;
>  
> @@ -498,9 +517,7 @@ static NTSTATUS smb1srv_session_local_allocate_id(struct db_context *db,
>  			id = highest_id;
>  		}
>  
> -		key = smbXsrv_session_local_id_to_key(id, key_buf);
> -
> -		rec = dbwrap_fetch_locked(db, mem_ctx, key);
> +		rec = smbXsrv_session_local_fetch_locked(db, id, mem_ctx);
>  		if (rec == NULL) {
>  			return NT_STATUS_INSUFFICIENT_RESOURCES;
>  		}
> @@ -550,16 +567,12 @@ static NTSTATUS smb1srv_session_local_allocate_id(struct db_context *db,
>  
>  	if (NT_STATUS_IS_OK(state.status)) {
>  		uint32_t id;
> -		uint8_t key_buf[SMBXSRV_SESSION_LOCAL_TDB_KEY_SIZE];
> -		TDB_DATA key;
>  		TDB_DATA val;
>  		struct db_record *rec = NULL;
>  
>  		id = state.useable_id;
>  
> -		key = smbXsrv_session_local_id_to_key(id, key_buf);
> -
> -		rec = dbwrap_fetch_locked(db, mem_ctx, key);
> +		rec = smbXsrv_session_local_fetch_locked(db, id, mem_ctx);
>  		if (rec == NULL) {
>  			return NT_STATUS_INSUFFICIENT_RESOURCES;
>  		}
> @@ -1206,8 +1219,6 @@ NTSTATUS smbXsrv_session_create(struct smbXsrv_connection *conn,
>  
>  	if (conn->protocol >= PROTOCOL_SMB2_02) {
>  		uint64_t id = global->session_global_id;
> -		uint8_t key_buf[SMBXSRV_SESSION_LOCAL_TDB_KEY_SIZE];
> -		TDB_DATA key;
>  
>  		global->connection_dialect = conn->smb2.server.dialect;
>  
> @@ -1221,10 +1232,10 @@ NTSTATUS smbXsrv_session_create(struct smbXsrv_connection *conn,
>  
>  		session->local_id = global->session_global_id;
>  
> -		key = smbXsrv_session_local_id_to_key(session->local_id, key_buf);
> -
> -		local_rec = dbwrap_fetch_locked(table->local.db_ctx,
> -						session, key);
> +		local_rec = smbXsrv_session_local_fetch_locked(
> +						table->local.db_ctx,
> +						session->local_id,
> +						session /* TALLOC_CTX */);
>  		if (local_rec == NULL) {
>  			TALLOC_FREE(session);
>  			return NT_STATUS_NO_MEMORY;
> @@ -1655,20 +1666,11 @@ NTSTATUS smbXsrv_session_logoff(struct smbXsrv_session *session)
>  
>  	local_rec = session->db_rec;
>  	if (local_rec == NULL) {
> -		uint8_t key_buf[SMBXSRV_SESSION_LOCAL_TDB_KEY_SIZE];
> -		TDB_DATA key;
> -
> -		key = smbXsrv_session_local_id_to_key(session->local_id,
> -						      key_buf);
> -
> -		local_rec = dbwrap_fetch_locked(table->local.db_ctx,
> -						session, key);
> +		local_rec = smbXsrv_session_local_fetch_locked(
> +						table->local.db_ctx,
> +						session->local_id,
> +						session /* TALLOC_CTX */);
>  		if (local_rec == NULL) {
> -			DEBUG(0, ("smbXsrv_session_logoff(0x%08x): "
> -				  "Failed to lock local key '%s'\n",
> -				  session->global->session_global_id,
> -				  hex_encode_talloc(local_rec, key.dptr,
> -						    key.dsize)));
>  			error = NT_STATUS_INTERNAL_ERROR;
>  		}
>  	}
> -- 
> 2.5.0
> 
> 
> From 0e97da3d9326c6872155c3fc1468504b405985df Mon Sep 17 00:00:00 2001
> From: Michael Adam <obnox at samba.org>
> Date: Sat, 27 Feb 2016 01:58:45 +0100
> Subject: [PATCH 5/5] smbXsrv_client: factor fetch-locking of global record
>  into function
> 
> Signed-off-by: Michael Adam <obnox at samba.org>
> ---
>  source3/smbd/smbXsrv_client.c | 62 +++++++++++++++++++++++--------------------
>  1 file changed, 33 insertions(+), 29 deletions(-)
> 
> diff --git a/source3/smbd/smbXsrv_client.c b/source3/smbd/smbXsrv_client.c
> index 0e21fc6..2dd4cad 100644
> --- a/source3/smbd/smbXsrv_client.c
> +++ b/source3/smbd/smbXsrv_client.c
> @@ -118,6 +118,28 @@ static TDB_DATA smbXsrv_client_global_id_to_key(const struct GUID *client_guid,
>  	return key;
>  }
>  
> +static struct db_record *smbXsrv_client_global_fetch_locked(
> +			struct db_context *db,
> +			const struct GUID *client_guid,
> +			TALLOC_CTX *mem_ctx)
> +{
> +	TDB_DATA key;
> +	uint8_t key_buf[SMBXSRV_CLIENT_GLOBAL_TDB_KEY_SIZE];
> +	struct db_record *rec = NULL;
> +
> +	key = smbXsrv_client_global_id_to_key(client_guid, key_buf);
> +
> +	rec = dbwrap_fetch_locked(db, mem_ctx, key);
> +
> +	if (rec == NULL) {
> +		DBG_DEBUG("Failed to lock guid [%s], key '%s'\n",
> +			  GUID_string(talloc_tos(), client_guid),
> +			  hex_encode_talloc(talloc_tos(), key.dptr, key.dsize));
> +	}
> +
> +	return rec;
> +}
> +
>  static NTSTATUS smbXsrv_client_table_create(TALLOC_CTX *mem_ctx,
>  					    struct messaging_context *msg_ctx,
>  					    uint32_t max_clients,
> @@ -252,18 +274,12 @@ NTSTATUS smb2srv_client_lookup_global(struct smbXsrv_client *client,
>  	struct smbXsrv_client_table *table = client->table;
>  	struct smbXsrv_client_global0 *global = NULL;
>  	bool is_free = false;
> -	uint8_t key_buf[SMBXSRV_CLIENT_GLOBAL_TDB_KEY_SIZE];
> -	TDB_DATA key;
>  	struct db_record *db_rec;
>  
> -	key = smbXsrv_client_global_id_to_key(&client_guid, key_buf);
> -
> -	db_rec = dbwrap_fetch_locked(table->global.db_ctx,
> -				     talloc_tos(), key);
> +	db_rec = smbXsrv_client_global_fetch_locked(table->global.db_ctx,
> +						    &client_guid,
> +						    talloc_tos());
>  	if (db_rec == NULL) {
> -		DBG_ERR("guid [%s]: Failed to lock key '%s'\n",
> -			GUID_string(talloc_tos(), &client_guid),
> -			hex_encode_talloc(talloc_tos(), key.dptr, key.dsize));
>  		return NT_STATUS_INTERNAL_DB_ERROR;
>  	}
>  
> @@ -681,8 +697,6 @@ NTSTATUS smbXsrv_client_update(struct smbXsrv_client *client)
>  {
>  	struct smbXsrv_client_table *table = client->table;
>  	NTSTATUS status;
> -	uint8_t key_buf[SMBXSRV_CLIENT_GLOBAL_TDB_KEY_SIZE];
> -	TDB_DATA key;
>  
>  	if (client->global->db_rec != NULL) {
>  		DBG_ERR("guid [%s]: Called with db_rec != NULL'\n",
> @@ -691,15 +705,11 @@ NTSTATUS smbXsrv_client_update(struct smbXsrv_client *client)
>  		return NT_STATUS_INTERNAL_ERROR;
>  	}
>  
> -	key = smbXsrv_client_global_id_to_key(&client->global->client_guid,
> -					      key_buf);
> -
> -	client->global->db_rec = dbwrap_fetch_locked(table->global.db_ctx,
> -						     client->global, key);
> +	client->global->db_rec = smbXsrv_client_global_fetch_locked(
> +					table->global.db_ctx,
> +					&client->global->client_guid,
> +					client->global /* TALLOC_CTX */);
>  	if (client->global->db_rec == NULL) {
> -		DBG_ERR("guid [%s]: Failed to lock key '%s'\n",
> -			GUID_string(talloc_tos(), &client->global->client_guid),
> -			hex_encode_talloc(talloc_tos(), key.dptr, key.dsize));
>  		return NT_STATUS_INTERNAL_DB_ERROR;
>  	}
>  
> @@ -730,8 +740,6 @@ NTSTATUS smbXsrv_client_remove(struct smbXsrv_client *client)
>  {
>  	struct smbXsrv_client_table *table = client->table;
>  	NTSTATUS status;
> -	uint8_t key_buf[SMBXSRV_CLIENT_GLOBAL_TDB_KEY_SIZE];
> -	TDB_DATA key;
>  
>  	if (client->global->db_rec != NULL) {
>  		DBG_ERR("client_guid[%s]: Called with db_rec != NULL'\n",
> @@ -743,15 +751,11 @@ NTSTATUS smbXsrv_client_remove(struct smbXsrv_client *client)
>  		return NT_STATUS_OK;
>  	}
>  
> -	key = smbXsrv_client_global_id_to_key(&client->global->client_guid,
> -					      key_buf);
> -
> -	client->global->db_rec = dbwrap_fetch_locked(table->global.db_ctx,
> -						     client->global, key);
> +	client->global->db_rec = smbXsrv_client_global_fetch_locked(
> +					table->global.db_ctx,
> +					&client->global->client_guid,
> +					client->global /* TALLOC_CTX */);
>  	if (client->global->db_rec == NULL) {
> -		DBG_ERR("client_guid[%s]: Failed to lock key '%s'\n",
> -			GUID_string(talloc_tos(), &client->global->client_guid),
> -			hex_encode_talloc(talloc_tos(), key.dptr, key.dsize));
>  		return NT_STATUS_INTERNAL_DB_ERROR;
>  	}
>  
> -- 
> 2.5.0
> 






More information about the samba-technical mailing list