[PATCH] Some small cleanups
Jeremy Allison
jra at samba.org
Fri Aug 17 16:23:48 UTC 2018
On Fri, Aug 17, 2018 at 11:23:56AM +0200, Volker Lendecke via samba-technical wrote:
> On Fri, Aug 17, 2018 at 11:16:08AM +0200, Volker Lendecke via samba-technical wrote:
> > Hi!
> >
> > Some typos and an API clarification.
> >
> > Review appreciated!
>
> ENOPATCH
LGTM and pushed (with Rowland's update).
FYI, I *really* like the:
dbwrap: Clarify db_open_watched API
change. That really does help clarify
the API the right way, thanks.
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
>
> Meet us at Storage Developer Conference (SDC)
> Santa Clara, CA USA, September 24th-27th 2018
> From 74ade76dc2ba4d88b0db49afed796a17f4085593 Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Tue, 7 Aug 2018 15:09:04 +0200
> Subject: [PATCH 1/7] smbd: Enhance debugging in set_file_size
>
> I've stumbled over a case where VFS_FTRUNCATE wasn't called due to an
> unchanged size. Make that easier to detect. Also, get rid of an ancient
> cast to (double).
>
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
> source3/smbd/trans2.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c
> index 0ec20fe6337..c0f984704d2 100644
> --- a/source3/smbd/trans2.c
> +++ b/source3/smbd/trans2.c
> @@ -6355,7 +6355,9 @@ static NTSTATUS smb_set_file_size(connection_struct *conn,
> return NT_STATUS_OBJECT_NAME_NOT_FOUND;
> }
>
> - DEBUG(6,("smb_set_file_size: size: %.0f ", (double)size));
> + DBG_INFO("size: %"PRIu64", file_size_stat=%"PRIu64"\n",
> + (uint64_t)size,
> + get_file_size_stat(psbuf));
>
> if (size == get_file_size_stat(psbuf)) {
> return NT_STATUS_OK;
> --
> 2.11.0
>
>
> From ebc5b9768587bd3dcf86f84f602df3981913490f Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Thu, 26 Jul 2018 12:43:30 +0200
> Subject: [PATCH 2/7] smbd: Fix a typo
>
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
> source3/smbd/oplock.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/source3/smbd/oplock.c b/source3/smbd/oplock.c
> index 63623297d07..295ae1ff72d 100644
> --- a/source3/smbd/oplock.c
> +++ b/source3/smbd/oplock.c
> @@ -1153,7 +1153,7 @@ static void contend_level2_oplocks_begin_default(files_struct *fsp,
>
> /*
> * do_break_to_none() only operates on the
> - * locking.tdb and send network packets to
> + * locking.tdb and sends network packets to
> * the client. That doesn't require any
> * impersonation, so we just use the
> * raw tevent context here.
> --
> 2.11.0
>
>
> From 872a64d44e21f780bf5de21383e06306a8fd248c Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Thu, 26 Jul 2018 17:56:31 +0200
> Subject: [PATCH 3/7] smbd: Fix a typo
>
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
> source3/smbd/open.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/source3/smbd/open.c b/source3/smbd/open.c
> index db5eb4e459b..cbabea35157 100644
> --- a/source3/smbd/open.c
> +++ b/source3/smbd/open.c
> @@ -5005,7 +5005,7 @@ static NTSTATUS lease_match(connection_struct *conn,
> * Send the breaks and then return
> * SMB2_LEASE_NONE in the lease handle
> * to cause them to acknowledge the
> - * lease break. Consulatation with
> + * lease break. Consultation with
> * Microsoft engineering confirmed
> * this approach is safe.
> */
> --
> 2.11.0
>
>
> From c9f3eefb027f43db6662c5bcdc5ae5ff4754a9e5 Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Thu, 16 Aug 2018 11:34:36 +0200
> Subject: [PATCH 4/7] g_lock: Fix DEBUG messages
>
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
> source3/lib/g_lock.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/source3/lib/g_lock.c b/source3/lib/g_lock.c
> index de24b6c847b..9e7d683b470 100644
> --- a/source3/lib/g_lock.c
> +++ b/source3/lib/g_lock.c
> @@ -167,14 +167,14 @@ struct g_lock_ctx *g_lock_ctx_init(TALLOC_CTX *mem_ctx,
> DBWRAP_FLAG_NONE);
> TALLOC_FREE(db_path);
> if (backend == NULL) {
> - DEBUG(1, ("g_lock_init: Could not open g_lock.tdb\n"));
> + DBG_WARNING("Could not open g_lock.tdb\n");
> TALLOC_FREE(result);
> return NULL;
> }
>
> result->db = db_open_watched(result, backend, msg);
> if (result->db == NULL) {
> - DBG_WARNING("g_lock_init: db_open_watched failed\n");
> + DBG_WARNING("db_open_watched failed\n");
> TALLOC_FREE(result);
> return NULL;
> }
> --
> 2.11.0
>
>
> From e4bb33c7208157d99b421459aa013d0d75d06dce Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Mon, 30 Jul 2018 16:29:58 +0200
> Subject: [PATCH 5/7] dbwrap: Fix a typo
>
> ---
> lib/dbwrap/dbwrap.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/dbwrap/dbwrap.c b/lib/dbwrap/dbwrap.c
> index e1d792fa2f8..6a29306a68e 100644
> --- a/lib/dbwrap/dbwrap.c
> +++ b/lib/dbwrap/dbwrap.c
> @@ -457,7 +457,7 @@ struct tevent_req *dbwrap_parse_record_send(
>
> /*
> * Copy the key into our state ensuring the key data buffer is always
> - * available to the all dbwrap backend over the entire lifetime of the
> + * available to the all dbwrap backends over the entire lifetime of the
> * async request. Otherwise the caller might have free'd the key buffer.
> */
> if (key.dsize > sizeof(state->_keybuf)) {
> --
> 2.11.0
>
>
> From dc9f5122a66c218dec4c6a6a48b7ed28a241c00b Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Mon, 30 Jul 2018 13:00:22 +0200
> Subject: [PATCH 6/7] smbd: Fix a few DEBUG statements
>
> %u is not necessarily correct for uint32_t, avoid casts
>
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
> source3/locking/locking.c | 22 ++++++++++++++--------
> 1 file changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/source3/locking/locking.c b/source3/locking/locking.c
> index 208f7e2081d..b2681208cb8 100644
> --- a/source3/locking/locking.c
> +++ b/source3/locking/locking.c
> @@ -739,8 +739,10 @@ bool share_mode_stale_pid(struct share_mode_data *d, uint32_t idx)
> struct share_mode_entry *e;
>
> if (idx > d->num_share_modes) {
> - DEBUG(1, ("Asking for index %u, only %u around\n",
> - idx, (unsigned)d->num_share_modes));
> + DBG_WARNING("Asking for index %"PRIu32", "
> + "only %"PRIu32" around\n",
> + idx,
> + d->num_share_modes);
> return false;
> }
> e = &d->share_modes[idx];
> @@ -751,14 +753,18 @@ bool share_mode_stale_pid(struct share_mode_data *d, uint32_t idx)
> return true;
> }
> if (serverid_exists(&e->pid)) {
> - DEBUG(10, ("PID %s (index %u out of %u) still exists\n",
> - server_id_str_buf(e->pid, &tmp), idx,
> - (unsigned)d->num_share_modes));
> + DBG_DEBUG("PID %s (index %"PRIu32" out of %"PRIu32") "
> + "still exists\n",
> + server_id_str_buf(e->pid, &tmp),
> + idx,
> + d->num_share_modes);
> return false;
> }
> - DEBUG(10, ("PID %s (index %u out of %u) does not exist anymore\n",
> - server_id_str_buf(e->pid, &tmp), idx,
> - (unsigned)d->num_share_modes));
> + DBG_DEBUG("PID %s (index %"PRIu32" out of %"PRIu32") "
> + "does not exist anymore\n",
> + server_id_str_buf(e->pid, &tmp),
> + idx,
> + d->num_share_modes);
>
> e->stale = true;
>
> --
> 2.11.0
>
>
> From 0d64e21210029882962d64a8989099e00ea4a04f Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Thu, 16 Aug 2018 11:25:54 +0200
> Subject: [PATCH 7/7] dbwrap: Clarify db_open_watched API
>
> Point out in the API that "backend" talloc_moves into the watched
> database.
>
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
> source3/lib/dbwrap/dbwrap_watch.c | 8 ++++----
> source3/lib/dbwrap/dbwrap_watch.h | 2 +-
> source3/lib/g_lock.c | 2 +-
> source3/locking/share_mode_lock.c | 2 +-
> source3/smbd/smbXsrv_session.c | 2 +-
> source3/torture/test_dbwrap_do_locked.c | 2 +-
> source3/torture/test_dbwrap_watch.c | 4 ++--
> 7 files changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/source3/lib/dbwrap/dbwrap_watch.c b/source3/lib/dbwrap/dbwrap_watch.c
> index 3e91f46b428..45e17958b92 100644
> --- a/source3/lib/dbwrap/dbwrap_watch.c
> +++ b/source3/lib/dbwrap/dbwrap_watch.c
> @@ -738,7 +738,7 @@ static size_t dbwrap_watched_id(struct db_context *db, uint8_t *id,
> }
>
> struct db_context *db_open_watched(TALLOC_CTX *mem_ctx,
> - struct db_context *backend,
> + struct db_context **backend,
> struct messaging_context *msg)
> {
> struct db_context *db;
> @@ -757,9 +757,9 @@ struct db_context *db_open_watched(TALLOC_CTX *mem_ctx,
>
> ctx->msg = msg;
>
> - db->lock_order = backend->lock_order;
> - backend->lock_order = DBWRAP_LOCK_ORDER_NONE;
> - ctx->backend = talloc_move(ctx, &backend);
> + ctx->backend = talloc_move(ctx, backend);
> + db->lock_order = ctx->backend->lock_order;
> + ctx->backend->lock_order = DBWRAP_LOCK_ORDER_NONE;
>
> db->fetch_locked = dbwrap_watched_fetch_locked;
> db->do_locked = dbwrap_watched_do_locked;
> diff --git a/source3/lib/dbwrap/dbwrap_watch.h b/source3/lib/dbwrap/dbwrap_watch.h
> index e94378f2d5a..a836ca48e8a 100644
> --- a/source3/lib/dbwrap/dbwrap_watch.h
> +++ b/source3/lib/dbwrap/dbwrap_watch.h
> @@ -25,7 +25,7 @@
> #include "messages.h"
>
> struct db_context *db_open_watched(TALLOC_CTX *mem_ctx,
> - struct db_context *backend,
> + struct db_context **backend,
> struct messaging_context *msg);
> struct tevent_req *dbwrap_watched_watch_send(TALLOC_CTX *mem_ctx,
> struct tevent_context *ev,
> diff --git a/source3/lib/g_lock.c b/source3/lib/g_lock.c
> index 9e7d683b470..9904891a7af 100644
> --- a/source3/lib/g_lock.c
> +++ b/source3/lib/g_lock.c
> @@ -172,7 +172,7 @@ struct g_lock_ctx *g_lock_ctx_init(TALLOC_CTX *mem_ctx,
> return NULL;
> }
>
> - result->db = db_open_watched(result, backend, msg);
> + result->db = db_open_watched(result, &backend, msg);
> if (result->db == NULL) {
> DBG_WARNING("db_open_watched failed\n");
> TALLOC_FREE(result);
> diff --git a/source3/locking/share_mode_lock.c b/source3/locking/share_mode_lock.c
> index 9314735ceda..90b39d303fa 100644
> --- a/source3/locking/share_mode_lock.c
> +++ b/source3/locking/share_mode_lock.c
> @@ -86,7 +86,7 @@ static bool locking_init_internal(bool read_only)
> return False;
> }
>
> - lock_db = db_open_watched(NULL, backend, server_messaging_context());
> + lock_db = db_open_watched(NULL, &backend, server_messaging_context());
> if (lock_db == NULL) {
> DBG_ERR("db_open_watched failed\n");
> TALLOC_FREE(backend);
> diff --git a/source3/smbd/smbXsrv_session.c b/source3/smbd/smbXsrv_session.c
> index cf537e7dc9d..0e71defaf45 100644
> --- a/source3/smbd/smbXsrv_session.c
> +++ b/source3/smbd/smbXsrv_session.c
> @@ -89,7 +89,7 @@ NTSTATUS smbXsrv_session_global_init(struct messaging_context *msg_ctx)
> return status;
> }
>
> - db_ctx = db_open_watched(NULL, backend, server_messaging_context());
> + db_ctx = db_open_watched(NULL, &backend, server_messaging_context());
> if (db_ctx == NULL) {
> TALLOC_FREE(backend);
> return NT_STATUS_NO_MEMORY;
> diff --git a/source3/torture/test_dbwrap_do_locked.c b/source3/torture/test_dbwrap_do_locked.c
> index 46b326bf163..2e5305d4d75 100644
> --- a/source3/torture/test_dbwrap_do_locked.c
> +++ b/source3/torture/test_dbwrap_do_locked.c
> @@ -97,7 +97,7 @@ bool run_dbwrap_do_locked1(int dummy)
> return false;
> }
>
> - db = db_open_watched(talloc_tos(), backend, msg);
> + db = db_open_watched(talloc_tos(), &backend, msg);
> if (db == NULL) {
> fprintf(stderr, "db_open_watched failed: %s\n",
> strerror(errno));
> diff --git a/source3/torture/test_dbwrap_watch.c b/source3/torture/test_dbwrap_watch.c
> index 97d5ea6393c..5ef6b105ca2 100644
> --- a/source3/torture/test_dbwrap_watch.c
> +++ b/source3/torture/test_dbwrap_watch.c
> @@ -56,7 +56,7 @@ bool run_dbwrap_watch1(int dummy)
> goto fail;
> }
>
> - db = db_open_watched(ev, backend, msg);
> + db = db_open_watched(ev, &backend, msg);
>
> rec = dbwrap_fetch_locked(db, db, key);
> if (rec == NULL) {
> @@ -153,7 +153,7 @@ bool run_dbwrap_watch2(int dummy)
> goto fail;
> }
>
> - db = db_open_watched(ev, backend, msg);
> + db = db_open_watched(ev, &backend, msg);
> if (db == NULL) {
> fprintf(stderr, "db_open_watched failed\n");
> goto fail;
> --
> 2.11.0
>
More information about the samba-technical
mailing list