[PATCH] Small simplification of open_files.idl
Jeremy Allison
jra at samba.org
Thu Jul 26 20:05:29 UTC 2018
On Thu, Jul 26, 2018 at 06:38:03AM +0200, Volker Lendecke via samba-technical wrote:
> Hi!
>
> ... along with two smaller ones. Review appreciated!
Oh nice cleanup, thanks ! Really like the removal of
share_mode_lease *lease.
RB+ and pushed !
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 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