[PATCH] A few cleanups
Volker Lendecke
Volker.Lendecke at SerNet.DE
Thu Sep 6 15:10:14 UTC 2018
Hi!
Attached find a few hopefully not too controversial cleanups.
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
Meet us at Storage Developer Conference (SDC)
Santa Clara, CA USA, September 24th-27th 2018
-------------- next part --------------
From 2751ce4b92499b07794b49a11342343c752b6130 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Thu, 26 Jul 2018 12:35:30 +0200
Subject: [PATCH 1/8] smbd: Factor out file_has_read_oplocks()
Why? This makes it clearer to me that we're not interested in the actual
number of read oplocks. We only want to know if there are any read
oplocks at all.
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/smbd/oplock.c | 30 ++++++++++++++++++++----------
1 file changed, 20 insertions(+), 10 deletions(-)
diff --git a/source3/smbd/oplock.c b/source3/smbd/oplock.c
index 1495269efb4..a23b87769e2 100644
--- a/source3/smbd/oplock.c
+++ b/source3/smbd/oplock.c
@@ -1052,6 +1052,23 @@ static void process_kernel_oplock_break(struct messaging_context *msg_ctx,
add_oplock_timeout_handler(fsp);
}
+static bool file_has_read_oplocks(struct files_struct *fsp)
+{
+ struct byte_range_lock *brl;
+ uint32_t num_read_oplocks = 0;
+
+ brl = brl_get_locks_readonly(fsp);
+ if (brl == NULL) {
+ return false;
+ }
+
+ num_read_oplocks = brl_num_read_oplocks(brl);
+
+ DBG_DEBUG("num_read_oplocks = %"PRIu32"\n", num_read_oplocks);
+
+ return (num_read_oplocks != 0);
+}
+
struct break_to_none_state {
struct smbd_server_connection *sconn;
struct file_id id;
@@ -1074,8 +1091,7 @@ static void contend_level2_oplocks_begin_default(files_struct *fsp,
struct smbd_server_connection *sconn = fsp->conn->sconn;
struct tevent_immediate *im;
struct break_to_none_state *state;
- struct byte_range_lock *brl;
- uint32_t num_read_oplocks = 0;
+ bool has_read_oplocks;
/*
* If this file is level II oplocked then we need
@@ -1092,14 +1108,8 @@ static void contend_level2_oplocks_begin_default(files_struct *fsp,
return;
}
- brl = brl_get_locks_readonly(fsp);
- if (brl != NULL) {
- num_read_oplocks = brl_num_read_oplocks(brl);
- }
-
- DEBUG(10, ("num_read_oplocks = %"PRIu32"\n", num_read_oplocks));
-
- if (num_read_oplocks == 0) {
+ has_read_oplocks = file_has_read_oplocks(fsp);
+ if (!has_read_oplocks) {
DEBUG(10, ("No read oplocks around\n"));
return;
}
--
2.11.0
From 85d2bfe233b3eaad9f8fadf0b00d80aee7ae77ef Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Thu, 26 Jul 2018 14:52:55 +0200
Subject: [PATCH 2/8] smbd: Simplify logic in smb2_lease_break_send
If/else if chains are hard to follow to me. Simplify the code by using
early returns.
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/smbd/smb2_break.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/source3/smbd/smb2_break.c b/source3/smbd/smb2_break.c
index 21fbef42dce..ec9c0f1376b 100644
--- a/source3/smbd/smb2_break.c
+++ b/source3/smbd/smb2_break.c
@@ -386,14 +386,16 @@ static struct tevent_req *smbd_smb2_lease_break_send(
status = NT_STATUS_OBJECT_NAME_NOT_FOUND;
DEBUG(10, ("No record for lease key found\n"));
}
- } else if (!NT_STATUS_IS_OK(lls.status)) {
- status = lls.status;
- } else if (lls.num_file_ids == 0) {
- status = NT_STATUS_OBJECT_NAME_NOT_FOUND;
+ tevent_req_nterror(req, status);
+ return tevent_req_post(req, ev);
}
- if (!NT_STATUS_IS_OK(status)) {
- tevent_req_nterror(req, status);
+ if (tevent_req_nterror(req, lls.status)) {
+ return tevent_req_post(req, ev);
+ }
+
+ if (lls.num_file_ids == 0) {
+ tevent_req_nterror(req, NT_STATUS_OBJECT_NAME_NOT_FOUND);
return tevent_req_post(req, ev);
}
--
2.11.0
From 1503907fa3932616553d5b0397138d0f5d12766c Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Thu, 26 Jul 2018 15:46:18 +0200
Subject: [PATCH 3/8] smbd: Simplify logic in fsp_lease_update
One if-condition is better than two
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/smbd/oplock.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/source3/smbd/oplock.c b/source3/smbd/oplock.c
index a23b87769e2..8073fbe3dca 100644
--- a/source3/smbd/oplock.c
+++ b/source3/smbd/oplock.c
@@ -400,11 +400,7 @@ bool fsp_lease_update(struct share_mode_lock *lck,
struct share_mode_lease *l = NULL;
idx = find_share_mode_lease(d, client_guid, &lease->lease.lease_key);
- if (idx != -1) {
- l = &d->leases[idx];
- }
-
- if (l == NULL) {
+ if (idx == -1) {
DEBUG(1, ("%s: Could not find lease entry\n", __func__));
TALLOC_FREE(lease->timeout);
lease->lease.lease_state = SMB2_LEASE_NONE;
@@ -413,6 +409,8 @@ bool fsp_lease_update(struct share_mode_lock *lck,
return false;
}
+ l = &d->leases[idx];
+
DEBUG(10,("%s: refresh lease state\n", __func__));
/* Ensure we're in sync with current lease state. */
--
2.11.0
From 8c7267e48819ae7ea2182c7261c338df67eb013f Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Fri, 27 Jul 2018 14:16:20 +0200
Subject: [PATCH 4/8] smbd: Simplify lease_match() a bit
This has been implicitly initialized to 0 with the explicit struct
initializer above.
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/smbd/open.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index cbabea35157..6bfd5ba1371 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -4924,8 +4924,6 @@ static NTSTATUS lease_match(connection_struct *conn,
state.file_existed = VALID_STAT(fname->st);
if (state.file_existed) {
state.id = vfs_file_id_from_sbuf(conn, &fname->st);
- } else {
- memset(&state.id, '\0', sizeof(state.id));
}
status = leases_db_parse(&sconn->client->connections->smb2.client.guid,
--
2.11.0
From af353878a771de701bb9b9d74f10c0d3cd505e79 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Mon, 30 Jul 2018 13:03:17 +0200
Subject: [PATCH 5/8] smbd: Simplify share_mode_stale_pid
This loop does not need to count valid share modes. A single valid one
is sufficient for keeping the delete token around
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/locking/locking.c | 20 ++++++++------------
1 file changed, 8 insertions(+), 12 deletions(-)
diff --git a/source3/locking/locking.c b/source3/locking/locking.c
index b2681208cb8..e9bec6d763e 100644
--- a/source3/locking/locking.c
+++ b/source3/locking/locking.c
@@ -769,24 +769,20 @@ bool share_mode_stale_pid(struct share_mode_data *d, uint32_t idx)
e->stale = true;
if (d->num_delete_tokens != 0) {
- uint32_t i, num_stale;
-
- /*
- * We cannot have any delete tokens
- * if there are no valid share modes.
- */
-
- num_stale = 0;
+ uint32_t i;
for (i=0; i<d->num_share_modes; i++) {
- if (d->share_modes[i].stale) {
- num_stale += 1;
+ bool valid = !d->share_modes[i].stale;
+ if (valid) {
+ break;
}
}
- if (num_stale == d->num_share_modes) {
+ if (i == d->num_share_modes) {
/*
- * No non-stale share mode found
+ * No valid (non-stale) share mode found, all
+ * who might have set the delete token are
+ * gone.
*/
TALLOC_FREE(d->delete_tokens);
d->num_delete_tokens = 0;
--
2.11.0
From 8bf2b45dd418a553950d8e8b87e4831b801ea0db Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Mon, 30 Jul 2018 13:21:26 +0200
Subject: [PATCH 6/8] smbd: Simplify logic in remove_stale_share_mode_entries
To me, an early "continue" is easier to follow than a "else".
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/locking/locking.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/source3/locking/locking.c b/source3/locking/locking.c
index e9bec6d763e..91e32ced304 100644
--- a/source3/locking/locking.c
+++ b/source3/locking/locking.c
@@ -805,9 +805,9 @@ void remove_stale_share_mode_entries(struct share_mode_data *d)
struct share_mode_entry *m = d->share_modes;
m[i] = m[d->num_share_modes-1];
d->num_share_modes -= 1;
- } else {
- i += 1;
+ continue;
}
+ i += 1;
}
}
--
2.11.0
From 1cbf8b8769524ee8b0f2190e6a7346ba0793be7a Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 5 Sep 2018 11:31:10 +0200
Subject: [PATCH 7/8] vfs_fruit: fix an uninitialized variable error
clang does not recognize "smb_panic" as an "exit()" equivalent
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/modules/vfs_fruit.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index 1102059bbee..894c361a05f 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -2331,7 +2331,7 @@ static off_t access_to_netatalk_brl(enum apple_fork fork_type,
static off_t denymode_to_netatalk_brl(enum apple_fork fork_type,
uint32_t deny_mode)
{
- off_t offset;
+ off_t offset = 0;
switch (deny_mode) {
case DENY_READ:
--
2.11.0
From bd5c6e227e454b02a25d065a60d5c7f766fb3ed1 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Fri, 31 Aug 2018 06:09:48 +0200
Subject: [PATCH 8/8] dbwrap: Remove a pointless "return;"
Signed-off-by: Volker Lendecke <vl at samba.org>
---
lib/dbwrap/dbwrap.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/lib/dbwrap/dbwrap.c b/lib/dbwrap/dbwrap.c
index a214a4e9a6e..79c83b1a5e2 100644
--- a/lib/dbwrap/dbwrap.c
+++ b/lib/dbwrap/dbwrap.c
@@ -505,7 +505,6 @@ static void dbwrap_parse_record_done(struct tevent_req *subreq)
}
tevent_req_done(req);
- return;
}
NTSTATUS dbwrap_parse_record_recv(struct tevent_req *req)
--
2.11.0
More information about the samba-technical
mailing list