[PATCH] A few cleanups around share_mode_data
Jeremy Allison
jra at samba.org
Mon Sep 10 17:34:39 UTC 2018
On Mon, Sep 10, 2018 at 04:10:50PM +0200, Volker Lendecke via samba-technical wrote:
> Hi!
>
> Review appreciated! Overall 30 lines less... :-)
LGTM, Nice cleanups thanks ! 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
>
> Meet us at Storage Developer Conference (SDC)
> Santa Clara, CA USA, September 24th-27th 2018
> From fb0678dc0d3e3b02b831918110f189b946c3b222 Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Sat, 8 Sep 2018 12:45:54 +0200
> Subject: [PATCH 1/5] smbd: Remove an unneeded #include
>
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
> source3/smbd/reply.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c
> index 5afe57d..4c6456c 100644
> --- a/source3/smbd/reply.c
> +++ b/source3/smbd/reply.c
> @@ -33,7 +33,6 @@
> #include "fake_file.h"
> #include "rpc_client/rpc_client.h"
> #include "../librpc/gen_ndr/ndr_spoolss_c.h"
> -#include "../librpc/gen_ndr/open_files.h"
> #include "rpc_client/cli_spoolss.h"
> #include "rpc_client/init_spoolss.h"
> #include "rpc_server/rpc_ncacn_np.h"
> --
> 1.9.1
>
>
> From 30332b8824516e35367bfe9bf0a2426e52661f6a Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Sat, 8 Sep 2018 13:44:30 +0200
> Subject: [PATCH 2/5] smbd: Simplify close_directory()
>
> Same patch as in 8541829a9ab20c7fa8c
>
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
> source3/smbd/close.c | 36 ++++++++++++++++++++----------------
> 1 file changed, 20 insertions(+), 16 deletions(-)
>
> diff --git a/source3/smbd/close.c b/source3/smbd/close.c
> index 7820fff..288415a 100644
> --- a/source3/smbd/close.c
> +++ b/source3/smbd/close.c
> @@ -1153,23 +1153,27 @@ static NTSTATUS close_directory(struct smb_request *req, files_struct *fsp,
> * case, then don't delete. If all opens are POSIX delete now. */
> for (i=0; i<lck->data->num_share_modes; i++) {
> struct share_mode_entry *e = &lck->data->share_modes[i];
> - if (is_valid_share_mode_entry(e) &&
> - e->name_hash == fsp->name_hash) {
> - if ((fsp->posix_flags & FSP_POSIX_FLAGS_OPEN) &&
> - (e->flags & SHARE_MODE_FLAG_POSIX_OPEN))
> - {
> - continue;
> - }
> - if (serverid_equal(&self, &e->pid) &&
> - (e->share_file_id == fsp->fh->gen_id)) {
> - continue;
> - }
> - if (share_mode_stale_pid(lck->data, i)) {
> - continue;
> - }
> - delete_dir = False;
> - break;
> +
> + if (!is_valid_share_mode_entry(e)) {
> + continue;
> + }
> + if (e->name_hash != fsp->name_hash) {
> + continue;
> }
> + if ((fsp->posix_flags & FSP_POSIX_FLAGS_OPEN) &&
> + (e->flags & SHARE_MODE_FLAG_POSIX_OPEN))
> + {
> + continue;
> + }
> + if (serverid_equal(&self, &e->pid) &&
> + (e->share_file_id == fsp->fh->gen_id)) {
> + continue;
> + }
> + if (share_mode_stale_pid(lck->data, i)) {
> + continue;
> + }
> + delete_dir = False;
> + break;
> }
> }
>
> --
> 1.9.1
>
>
> From b666d6826ea3881e4aaf738dcbfdc44b6cc5649e Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Sat, 8 Sep 2018 13:50:46 +0200
> Subject: [PATCH 3/5] smbd: Remove an unneeded #include
>
> ndr_open_files already includes open_files.h
>
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
> source3/smbd/oplock.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/source3/smbd/oplock.c b/source3/smbd/oplock.c
> index 8073fbe..2faad78 100644
> --- a/source3/smbd/oplock.c
> +++ b/source3/smbd/oplock.c
> @@ -25,7 +25,6 @@
> #include "smbd/smbd.h"
> #include "smbd/globals.h"
> #include "messages.h"
> -#include "../librpc/gen_ndr/open_files.h"
> #include "../librpc/gen_ndr/ndr_open_files.h"
>
> /*
> --
> 1.9.1
>
>
> From 6bd1c8a2b5657fcd7e2ffc0f1ac68326ab7f7541 Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Sat, 8 Sep 2018 16:58:36 +0200
> Subject: [PATCH 4/5] smbd: Factor out "has_other_nonposix_opens"
>
> This is exactly the same in both file and directory cases
>
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
> source3/smbd/close.c | 93 ++++++++++++++++++++--------------------------------
> source3/smbd/proto.h | 3 ++
> 2 files changed, 38 insertions(+), 58 deletions(-)
>
> diff --git a/source3/smbd/close.c b/source3/smbd/close.c
> index 288415a..742b3f0 100644
> --- a/source3/smbd/close.c
> +++ b/source3/smbd/close.c
> @@ -233,6 +233,39 @@ NTSTATUS delete_all_streams(connection_struct *conn,
> return status;
> }
>
> +bool has_other_nonposix_opens(struct share_mode_lock *lck,
> + struct files_struct *fsp,
> + struct server_id self)
> +{
> + struct share_mode_data *data = lck->data;
> + uint32_t i;
> +
> + for (i=0; i<data->num_share_modes; i++) {
> + struct share_mode_entry *e = &data->share_modes[i];
> +
> + if (!is_valid_share_mode_entry(e)) {
> + continue;
> + }
> + if (e->name_hash != fsp->name_hash) {
> + continue;
> + }
> + if ((fsp->posix_flags & FSP_POSIX_FLAGS_OPEN) &&
> + (e->flags & SHARE_MODE_FLAG_POSIX_OPEN)) {
> + continue;
> + }
> + if (serverid_equal(&self, &e->pid) &&
> + (e->share_file_id == fsp->fh->gen_id)) {
> + continue;
> + }
> + if (share_mode_stale_pid(data, i)) {
> + continue;
> + }
> + return true;
> + }
> +
> + return false;
> +}
> +
> /****************************************************************************
> Deal with removing a share mode on last close.
> ****************************************************************************/
> @@ -320,35 +353,7 @@ static NTSTATUS close_remove_share_mode(files_struct *fsp,
>
> delete_file = is_delete_on_close_set(lck, fsp->name_hash);
>
> - if (delete_file) {
> - int i;
> - /* See if others still have the file open via this pathname.
> - If this is the case, then don't delete. If all opens are
> - POSIX delete now. */
> - for (i=0; i<lck->data->num_share_modes; i++) {
> - struct share_mode_entry *e = &lck->data->share_modes[i];
> -
> - if (!is_valid_share_mode_entry(e)) {
> - continue;
> - }
> - if (e->name_hash != fsp->name_hash) {
> - continue;
> - }
> - if ((fsp->posix_flags & FSP_POSIX_FLAGS_OPEN)
> - && (e->flags & SHARE_MODE_FLAG_POSIX_OPEN)) {
> - continue;
> - }
> - if (serverid_equal(&self, &e->pid) &&
> - (e->share_file_id == fsp->fh->gen_id)) {
> - continue;
> - }
> - if (share_mode_stale_pid(lck->data, i)) {
> - continue;
> - }
> - delete_file = False;
> - break;
> - }
> - }
> + delete_file &= !has_other_nonposix_opens(lck, fsp, self);
>
> /*
> * NT can set delete_on_close of the last open
> @@ -1147,35 +1152,7 @@ static NTSTATUS close_directory(struct smb_request *req, files_struct *fsp,
> delete_dir = get_delete_on_close_token(lck, fsp->name_hash,
> &del_nt_token, &del_token);
>
> - if (delete_dir) {
> - int i;
> - /* See if others still have the dir open. If this is the
> - * case, then don't delete. If all opens are POSIX delete now. */
> - for (i=0; i<lck->data->num_share_modes; i++) {
> - struct share_mode_entry *e = &lck->data->share_modes[i];
> -
> - if (!is_valid_share_mode_entry(e)) {
> - continue;
> - }
> - if (e->name_hash != fsp->name_hash) {
> - continue;
> - }
> - if ((fsp->posix_flags & FSP_POSIX_FLAGS_OPEN) &&
> - (e->flags & SHARE_MODE_FLAG_POSIX_OPEN))
> - {
> - continue;
> - }
> - if (serverid_equal(&self, &e->pid) &&
> - (e->share_file_id == fsp->fh->gen_id)) {
> - continue;
> - }
> - if (share_mode_stale_pid(lck->data, i)) {
> - continue;
> - }
> - delete_dir = False;
> - break;
> - }
> - }
> + delete_dir &= !has_other_nonposix_opens(lck, fsp, self);
>
> if ((close_type == NORMAL_CLOSE || close_type == SHUTDOWN_CLOSE) &&
> delete_dir) {
> diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
> index 2a41d9d..5399c5a 100644
> --- a/source3/smbd/proto.h
> +++ b/source3/smbd/proto.h
> @@ -146,6 +146,9 @@ NTSTATUS delete_all_streams(connection_struct *conn,
> bool recursive_rmdir(TALLOC_CTX *ctx,
> connection_struct *conn,
> struct smb_filename *smb_dname);
> +bool has_other_nonposix_opens(struct share_mode_lock *lck,
> + struct files_struct *fsp,
> + struct server_id self);
>
> /* The following definitions come from smbd/conn.c */
>
> --
> 1.9.1
>
>
> From b0dc40b66166a2e8f0c8605171672fe7fa66b263 Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Sat, 8 Sep 2018 18:05:57 +0200
> Subject: [PATCH 5/5] smbd: Use has_other_nonposix_opens in smb_posix_unlink
>
> Almost the same code as in close.c. has_other_nonposix_opens() is a bit
> more general, but the purpose is the same.
>
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
> source3/smbd/trans2.c | 30 +++++++++---------------------
> 1 file changed, 9 insertions(+), 21 deletions(-)
>
> diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c
> index c0f9847..0003c36 100644
> --- a/source3/smbd/trans2.c
> +++ b/source3/smbd/trans2.c
> @@ -32,7 +32,6 @@
> #include "../libcli/auth/libcli_auth.h"
> #include "../librpc/gen_ndr/xattr.h"
> #include "../librpc/gen_ndr/ndr_security.h"
> -#include "../librpc/gen_ndr/open_files.h"
> #include "libcli/security/security.h"
> #include "trans2.h"
> #include "auth.h"
> @@ -41,6 +40,7 @@
> #include "printing.h"
> #include "lib/util_ea.h"
> #include "lib/readdir_attr.h"
> +#include "messages.h"
>
> #define DIR_ENTRY_SAFETY_MARGIN 4096
>
> @@ -8309,14 +8309,15 @@ static NTSTATUS smb_posix_unlink(connection_struct *conn,
> int total_data,
> struct smb_filename *smb_fname)
> {
> + struct server_id self = messaging_server_id(conn->sconn->msg_ctx);
> NTSTATUS status = NT_STATUS_OK;
> files_struct *fsp = NULL;
> uint16_t flags = 0;
> char del = 1;
> int info = 0;
> int create_options = 0;
> - int i;
> struct share_mode_lock *lck = NULL;
> + bool other_nonposix_opens;
>
> if (total_data < 2) {
> return NT_STATUS_INVALID_PARAMETER;
> @@ -8379,25 +8380,12 @@ static NTSTATUS smb_posix_unlink(connection_struct *conn,
> return NT_STATUS_INVALID_PARAMETER;
> }
>
> - /*
> - * See if others still have the file open. If this is the case, then
> - * don't delete. If all opens are POSIX delete we can set the delete
> - * on close disposition.
> - */
> - for (i=0; i<lck->data->num_share_modes; i++) {
> - struct share_mode_entry *e = &lck->data->share_modes[i];
> - if (is_valid_share_mode_entry(e)) {
> - if (e->flags & SHARE_MODE_FLAG_POSIX_OPEN) {
> - continue;
> - }
> - if (share_mode_stale_pid(lck->data, i)) {
> - continue;
> - }
> - /* Fail with sharing violation. */
> - TALLOC_FREE(lck);
> - close_file(req, fsp, NORMAL_CLOSE);
> - return NT_STATUS_SHARING_VIOLATION;
> - }
> + other_nonposix_opens = has_other_nonposix_opens(lck, fsp, self);
> + if (other_nonposix_opens) {
> + /* Fail with sharing violation. */
> + TALLOC_FREE(lck);
> + close_file(req, fsp, NORMAL_CLOSE);
> + return NT_STATUS_SHARING_VIOLATION;
> }
>
> /*
> --
> 1.9.1
>
More information about the samba-technical
mailing list