[PATCH] Small simplification of open_files.idl

Volker Lendecke Volker.Lendecke at SerNet.DE
Thu Jul 26 04:38:03 UTC 2018


Hi!

... along with two smaller ones. 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 9ea51acfac3caa5d35884d2fdef30b41a6aa71eb Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 25 Jul 2018 12:05:58 +0200
Subject: [PATCH 1/5] idmap: Make pointer initialization explicit

Took me a few seconds to find this NULL initializer

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

diff --git a/source3/winbindd/idmap_tdb_common.c b/source3/winbindd/idmap_tdb_common.c
index 48d87bb16cf..ceb663c0ddd 100644
--- a/source3/winbindd/idmap_tdb_common.c
+++ b/source3/winbindd/idmap_tdb_common.c
@@ -221,14 +221,13 @@ NTSTATUS idmap_tdb_common_set_mapping(struct idmap_domain * dom,
 	struct idmap_tdb_common_context *ctx;
 	struct idmap_tdb_common_set_mapping_context state;
 	NTSTATUS ret;
-	char *ksidstr, *kidstr;
+	char *ksidstr = NULL;
+	char *kidstr = NULL;
 
 	if (!map || !map->sid) {
 		return NT_STATUS_INVALID_PARAMETER;
 	}
 
-	ksidstr = kidstr = NULL;
-
 	/* TODO: should we filter a set_mapping using low/high filters ? */
 
 	ctx =
-- 
2.11.0


From 5740ba4ad1ce63a8bdd5a98d68a23188b3f0fc23 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 25 Jul 2018 15:48:01 +0200
Subject: [PATCH 2/5] popt: popt 1.16 needs -liconv

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 third_party/popt/wscript | 1 +
 1 file changed, 1 insertion(+)

diff --git a/third_party/popt/wscript b/third_party/popt/wscript
index eae8602ea6c..135bae83043 100644
--- a/third_party/popt/wscript
+++ b/third_party/popt/wscript
@@ -17,5 +17,6 @@ def build(bld):
     bld.SAMBA_LIBRARY('popt',
                       source='popt.c poptconfig.c popthelp.c poptint.c poptparse.c',
                       cflags=cflags,
+                      deps='iconv',
                       allow_warnings=True,
                       private_library=True)
-- 
2.11.0


From 03758b8397fda118a2c122acfa5ea77eae9b75c0 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 25 Jul 2018 16:56:35 +0200
Subject: [PATCH 3/5] smbd: Pass "share_mode_data" to share_entry_forall
 callback

Quite a bit of the contents have been passed explicitly anyway.

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/locking/proto.h                   |  8 ++---
 source3/locking/share_mode_lock.c         | 26 ++++++--------
 source3/rpc_server/srvsvc/srv_srvsvc_nt.c | 58 +++++++++++++++----------------
 source3/utils/status.c                    | 14 ++++----
 4 files changed, 49 insertions(+), 57 deletions(-)

diff --git a/source3/locking/proto.h b/source3/locking/proto.h
index 403729c934a..b615a4ae6d0 100644
--- a/source3/locking/proto.h
+++ b/source3/locking/proto.h
@@ -212,10 +212,10 @@ int share_mode_forall(int (*fn)(struct file_id fid,
 				const struct share_mode_data *data,
 				void *private_data),
 		      void *private_data);
-int share_entry_forall(int (*fn)(const struct share_mode_entry *,
-				 const struct file_id *id,
-				 const char *, const char *,
-				 const char *, void *),
+int share_entry_forall(int (*fn)(struct file_id fid,
+				 const struct share_mode_data *data,
+				 const struct share_mode_entry *entry,
+				 void *private_data),
 		      void *private_data);
 bool share_mode_cleanup_disconnected(struct file_id id,
 				     uint64_t open_persistent_id);
diff --git a/source3/locking/share_mode_lock.c b/source3/locking/share_mode_lock.c
index fce0c335ec3..f62a3b4ff3d 100644
--- a/source3/locking/share_mode_lock.c
+++ b/source3/locking/share_mode_lock.c
@@ -878,11 +878,9 @@ int share_mode_forall(int (*fn)(struct file_id fid,
 }
 
 struct share_entry_forall_state {
-	int (*fn)(const struct share_mode_entry *e,
-		  const struct file_id *id,
-		  const char *service_path,
-		  const char *base_name,
-		  const char *stream_name,
+	int (*fn)(struct file_id fid,
+		  const struct share_mode_data *data,
+		  const struct share_mode_entry *entry,
 		  void *private_data);
 	void *private_data;
 };
@@ -897,11 +895,9 @@ static int share_entry_traverse_fn(struct file_id fid,
 	for (i=0; i<data->num_share_modes; i++) {
 		int ret;
 
-		ret = state->fn(&data->share_modes[i],
-				&data->id,
-				data->servicepath,
-				data->base_name,
-				data->stream_name,
+		ret = state->fn(fid,
+				data,
+				&data->share_modes[i],
 				state->private_data);
 		if (ret != 0) {
 			return ret;
@@ -916,11 +912,11 @@ static int share_entry_traverse_fn(struct file_id fid,
  share mode system.
 ********************************************************************/
 
-int share_entry_forall(int (*fn)(const struct share_mode_entry *,
-				 const struct file_id *id,
-				 const char *, const char *,
-				 const char *, void *),
-		       void *private_data)
+int share_entry_forall(int (*fn)(struct file_id fid,
+				 const struct share_mode_data *data,
+				 const struct share_mode_entry *entry,
+				 void *private_data),
+		      void *private_data)
 {
 	struct share_entry_forall_state state = {
 		.fn = fn, .private_data = private_data };
diff --git a/source3/rpc_server/srvsvc/srv_srvsvc_nt.c b/source3/rpc_server/srvsvc/srv_srvsvc_nt.c
index e1963a40a0c..7138f126be6 100644
--- a/source3/rpc_server/srvsvc/srv_srvsvc_nt.c
+++ b/source3/rpc_server/srvsvc/srv_srvsvc_nt.c
@@ -81,11 +81,9 @@ struct share_conn_stat {
 /*******************************************************************
 ********************************************************************/
 
-static int enum_file_fn(const struct share_mode_entry *e,
-			const struct file_id *id,
-			const char *sharepath,
-			const char *fname,
-			const char *sname,
+static int enum_file_fn(struct file_id id,
+			const struct share_mode_data *d,
+			const struct share_mode_entry *e,
 			void *private_data)
 {
 	struct file_enum_count *fenum =
@@ -124,19 +122,25 @@ static int enum_file_fn(const struct share_mode_entry *e,
 	/* need to count the number of locks on a file */
 
 	ZERO_STRUCT( fsp );
-	fsp.file_id = *id;
+	fsp.file_id = id;
 
 	if ( (brl = brl_get_locks(talloc_tos(), &fsp)) != NULL ) {
 		num_locks = brl_num_locks(brl);
 		TALLOC_FREE(brl);
 	}
 
-	if ( strcmp( fname, "." ) == 0 ) {
-		fullpath = talloc_asprintf(fenum->ctx, "C:%s", sharepath );
+	if ( strcmp(d->base_name, "." ) == 0 ) {
+		fullpath = talloc_asprintf(
+			fenum->ctx,
+			"C:%s",
+			d->servicepath);
 	} else {
-		fullpath = talloc_asprintf(fenum->ctx, "C:%s/%s%s",
-					   sharepath, fname,
-					   sname ? sname : "");
+		fullpath = talloc_asprintf(
+			fenum->ctx,
+			"C:%s/%s%s",
+			d->servicepath,
+			d->base_name,
+			(d->stream_name != NULL) ? d->stream_name : "");
 	}
 	if (!fullpath) {
 		return 0;
@@ -841,11 +845,9 @@ static WERROR init_srv_sess_info_0(struct pipes_struct *p,
  * find out the session on which this file is open and bump up its count
  **********************************************************************/
 
-static int count_sess_files_fn(const struct share_mode_entry *e,
-			       const struct file_id *id,
-			       const char *sharepath,
-			       const char *fname,
-			       const char *sname,
+static int count_sess_files_fn(struct file_id fid,
+			       const struct share_mode_data *d,
+			       const struct share_mode_entry *e,
 			       void *data)
 {
 	struct sess_file_info *info = data;
@@ -969,18 +971,16 @@ static WERROR init_srv_sess_info_1(struct pipes_struct *p,
  find the share connection on which this open exists.
  ********************************************************************/
 
-static int share_file_fn(const struct share_mode_entry *e,
-			 const struct file_id *id,
-			 const char *sharepath,
-			 const char *fname,
-			 const char *sname,
+static int share_file_fn(struct file_id fid,
+			 const struct share_mode_data *d,
+			 const struct share_mode_entry *e,
 			 void *data)
 {
 	struct share_file_stat *sfs = data;
 	uint32_t i;
 	uint32_t offset = sfs->total_entries - sfs->resp_entries;
 
-	if (strequal(sharepath, sfs->in_sharepath)) {
+	if (strequal(d->servicepath, sfs->in_sharepath)) {
 		for (i=0; i < sfs->resp_entries; i++) {
 			if (serverid_equal(&e->pid, &sfs->svrid_arr[offset + i])) {
 				sfs->netconn_arr[i].num_open ++;
@@ -2681,11 +2681,9 @@ struct enum_file_close_state {
 	struct messaging_context *msg_ctx;
 };
 
-static int enum_file_close_fn(const struct share_mode_entry *e,
-			      const struct file_id *id,
-			      const char *sharepath,
-			      const char *fname,
-			      const char *sname,
+static int enum_file_close_fn(struct file_id id,
+			      const struct share_mode_data *d,
+			      const struct share_mode_entry *e,
 			      void *private_data)
 {
 	char msg[MSG_SMB_SHARE_MODE_ENTRY_SIZE];
@@ -2702,10 +2700,10 @@ static int enum_file_close_fn(const struct share_mode_entry *e,
 	}
 
 	/* Ok - send the close message. */
-	DBG_DEBUG("request to close file %s, %s\n", sharepath,
-		  share_mode_str(talloc_tos(), 0, id, e));
+	DBG_DEBUG("request to close file %s, %s\n", d->servicepath,
+		  share_mode_str(talloc_tos(), 0, &id, e));
 
-	share_mode_entry_to_message(msg, id, e);
+	share_mode_entry_to_message(msg, &id, e);
 
 	state->r->out.result = ntstatus_to_werror(
 		messaging_send_buf(state->msg_ctx,
diff --git a/source3/utils/status.c b/source3/utils/status.c
index d04efedee3f..9f8729c2910 100644
--- a/source3/utils/status.c
+++ b/source3/utils/status.c
@@ -116,12 +116,10 @@ static bool Ucrit_addPid( struct server_id pid )
 	return True;
 }
 
-static int print_share_mode(const struct share_mode_entry *e,
-			    const struct file_id *id,
-			    const char *sharepath,
-			    const char *fname,
-			    const char *sname,
-			    void *dummy)
+static int print_share_mode(struct file_id fid,
+			    const struct share_mode_data *d,
+			    const struct share_mode_entry *e,
+			    void *private_data)
 {
 	static int count;
 
@@ -195,8 +193,8 @@ static int print_share_mode(const struct share_mode_entry *e,
 		}
 
 		d_printf(" %s   %s%s   %s",
-			 sharepath, fname,
-			 sname ? sname : "",
+			 d->servicepath, d->base_name,
+			 (d->stream_name != NULL) ? d->stream_name : "",
 			 time_to_asc((time_t)e->time.tv_sec));
 	}
 
-- 
2.11.0


From 46abc841ed5eb9ba9498ad672907fbe274cb8ca3 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 25 Jul 2018 16:59:53 +0200
Subject: [PATCH 4/5] smbstatus: Use share_mode_data->leases

This is the only user of share_mode_entry->lease

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/utils/status.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/source3/utils/status.c b/source3/utils/status.c
index 9f8729c2910..811eb7b492b 100644
--- a/source3/utils/status.c
+++ b/source3/utils/status.c
@@ -180,7 +180,8 @@ static int print_share_mode(struct file_id fid,
 		} else if (e->op_type & LEVEL_II_OPLOCK) {
 			d_printf("LEVEL_II        ");
 		} else if (e->op_type == LEASE_OPLOCK) {
-			uint32_t lstate = e->lease->current_state;
+			struct share_mode_lease *l = &d->leases[e->lease_idx];
+			uint32_t lstate = l->current_state;
 			d_printf("LEASE(%s%s%s)%s%s%s      ",
 				 (lstate & SMB2_LEASE_READ)?"R":"",
 				 (lstate & SMB2_LEASE_WRITE)?"W":"",
-- 
2.11.0


From 104bbb32f69ba94026044502e4b9edb888c26746 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 25 Jul 2018 17:05:34 +0200
Subject: [PATCH 5/5] smbd: Remove "share_mode_entry->lease"

smbstatus was the only user, and this could be solved by adapting
share_entry_forall.

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/librpc/idl/open_files.idl |  1 -
 source3/locking/locking.c         | 10 ++--------
 source3/locking/share_mode_lock.c | 16 ++--------------
 3 files changed, 4 insertions(+), 23 deletions(-)

diff --git a/source3/librpc/idl/open_files.idl b/source3/librpc/idl/open_files.idl
index 8d652a9fa01..c07a903878e 100644
--- a/source3/librpc/idl/open_files.idl
+++ b/source3/librpc/idl/open_files.idl
@@ -61,7 +61,6 @@ interface open_files
 		 * to store this share_mode_entry on disk.
 		 */
 		[skip] boolean8	stale;
-		[ignore] share_mode_lease *lease;
 	} share_mode_entry;
 
 	typedef [public] struct {
diff --git a/source3/locking/locking.c b/source3/locking/locking.c
index e962fee89ab..208f7e2081d 100644
--- a/source3/locking/locking.c
+++ b/source3/locking/locking.c
@@ -713,7 +713,6 @@ static void remove_share_mode_lease(struct share_mode_data *d,
 	for (i=0; i<d->num_share_modes; i++) {
 		if (d->share_modes[i].lease_idx == d->num_leases) {
 			d->share_modes[i].lease_idx = lease_idx;
-			d->share_modes[i].lease = &d->leases[lease_idx];
 		}
 	}
 
@@ -816,14 +815,10 @@ bool set_share_mode(struct share_mode_lock *lck, struct files_struct *fsp,
 {
 	struct share_mode_data *d = lck->data;
 	struct share_mode_entry *tmp, *e;
-	struct share_mode_lease *lease = NULL;
 
-	if (lease_idx == UINT32_MAX) {
-		lease = NULL;
-	} else if (lease_idx >= d->num_leases) {
+	if ((lease_idx != UINT32_MAX) &&
+	    (lease_idx >= d->num_leases)) {
 		return false;
-	} else {
-		lease = &d->leases[lease_idx];
 	}
 
 	tmp = talloc_realloc(d, d->share_modes, struct share_mode_entry,
@@ -844,7 +839,6 @@ bool set_share_mode(struct share_mode_lock *lck, struct files_struct *fsp,
 	e->op_mid = mid;
 	e->op_type = op_type;
 	e->lease_idx = lease_idx;
-	e->lease = lease;
 	e->time.tv_sec = fsp->open_time.tv_sec;
 	e->time.tv_usec = fsp->open_time.tv_usec;
 	e->share_file_id = fsp->fh->gen_id;
diff --git a/source3/locking/share_mode_lock.c b/source3/locking/share_mode_lock.c
index f62a3b4ff3d..9314735ceda 100644
--- a/source3/locking/share_mode_lock.c
+++ b/source3/locking/share_mode_lock.c
@@ -329,17 +329,7 @@ static struct share_mode_data *parse_share_modes(TALLOC_CTX *mem_ctx,
 	 */
 
 	for (i=0; i<d->num_share_modes; i++) {
-		struct share_mode_entry *e = &d->share_modes[i];
-
-		e->stale = false;
-		e->lease = NULL;
-		if (e->op_type != LEASE_OPLOCK) {
-			continue;
-		}
-		if (e->lease_idx >= d->num_leases) {
-			continue;
-		}
-		e->lease = &d->leases[e->lease_idx];
+		d->share_modes[i].stale = false;
 	}
 	d->modified = false;
 	d->fresh = false;
@@ -836,9 +826,7 @@ static int share_mode_traverse_fn(struct db_record *rec, void *_state)
 	}
 
 	for (i=0; i<d->num_share_modes; i++) {
-		struct share_mode_entry *entry = &d->share_modes[i];
-		entry->stale = false; /* [skip] in idl */
-		entry->lease = &d->leases[entry->lease_idx];
+		d->share_modes[i].stale = false;
 	}
 
 	if (DEBUGLEVEL > 10) {
-- 
2.11.0



More information about the samba-technical mailing list