[PATCH] A few cleanups for dbwrap, dbwrap_tdb and dbwrap_ctdb
Ralph Böhme
slow at samba.org
Tue Sep 11 20:38:06 UTC 2018
On Tue, Sep 11, 2018 at 06:36:01PM +0200, Ralph Böhme wrote:
>On Tue, Sep 11, 2018 at 08:14:05AM +0200, Ralph Böhme via samba-technical wrote:
>>Attached are a few cleanup patches for dbwrap, including two minor
>>fixes in the dbwrap_ctdb backend for returning the correct record
>>count traverses.
>
>autobuild:
>
><https://gitlab.com/samba-team/devel/samba/pipelines/29934027>
...failed. Sorry, missed a fixup while reordering patches from my WIP branches.
Updated patchset attached.
autobuild:
https://gitlab.com/samba-team/devel/samba/pipelines/29949827
-slow
--
Ralph Boehme, Samba Team https://samba.org/
Samba Developer, SerNet GmbH https://sernet.de/en/samba/
GPG Key Fingerprint: FAE2 C608 8A24 2520 51C5
59E4 AA1E 9B71 2639 9E46
-------------- next part --------------
From e9687bf8affa9f2fbdbb8cc94631d9c0520631b5 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Sat, 18 Nov 2017 08:56:09 +0100
Subject: [PATCH 01/11] dbwrap: move sockname variable and call to
lp_ctdbd_socket into context
sockname is only needed in a cluster.
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/lib/dbwrap/dbwrap_open.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/source3/lib/dbwrap/dbwrap_open.c b/source3/lib/dbwrap/dbwrap_open.c
index 33bf9696998..0a7edabdbc3 100644
--- a/source3/lib/dbwrap/dbwrap_open.c
+++ b/source3/lib/dbwrap/dbwrap_open.c
@@ -65,7 +65,6 @@ struct db_context *db_open(TALLOC_CTX *mem_ctx,
{
struct db_context *result = NULL;
const char *base;
- const char *sockname;
if (!DBWRAP_LOCK_ORDER_VALID(lock_order)) {
errno = EINVAL;
@@ -126,9 +125,10 @@ struct db_context *db_open(TALLOC_CTX *mem_ctx,
}
}
- sockname = lp_ctdbd_socket();
-
if (lp_clustering()) {
+ const char *sockname;
+
+ sockname = lp_ctdbd_socket();
if (!socket_exist(sockname)) {
DEBUG(1, ("ctdb socket does not exist - is ctdb not "
"running?\n"));
--
2.13.6
From 38d95a034c2f560386112e253c497280056a3868 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Sat, 18 Nov 2017 12:13:15 +0100
Subject: [PATCH 02/11] dbwrap: early return, removes an indentation level
No change in behaviour.
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/lib/dbwrap/dbwrap_open.c | 34 +++++++++++++++++-----------------
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/source3/lib/dbwrap/dbwrap_open.c b/source3/lib/dbwrap/dbwrap_open.c
index 0a7edabdbc3..c8dfd9103a8 100644
--- a/source3/lib/dbwrap/dbwrap_open.c
+++ b/source3/lib/dbwrap/dbwrap_open.c
@@ -65,6 +65,7 @@ struct db_context *db_open(TALLOC_CTX *mem_ctx,
{
struct db_context *result = NULL;
const char *base;
+ struct loadparm_context *lp_ctx = NULL;
if (!DBWRAP_LOCK_ORDER_VALID(lock_order)) {
errno = EINVAL;
@@ -159,27 +160,26 @@ struct db_context *db_open(TALLOC_CTX *mem_ctx,
}
return NULL;
}
+
+ return result;
}
}
- if (result == NULL) {
- struct loadparm_context *lp_ctx = loadparm_init_s3(mem_ctx, loadparm_s3_helpers());
+ lp_ctx = loadparm_init_s3(mem_ctx, loadparm_s3_helpers());
- if (hash_size == 0) {
- hash_size = lpcfg_tdb_hash_size(lp_ctx, name);
- }
- tdb_flags = lpcfg_tdb_flags(lp_ctx, tdb_flags);
-
- result = dbwrap_local_open(
- mem_ctx,
- name,
- hash_size,
- tdb_flags,
- open_flags,
- mode,
- lock_order,
- dbwrap_flags);
- talloc_unlink(mem_ctx, lp_ctx);
+ if (hash_size == 0) {
+ hash_size = lpcfg_tdb_hash_size(lp_ctx, name);
}
+ tdb_flags = lpcfg_tdb_flags(lp_ctx, tdb_flags);
+
+ result = dbwrap_local_open(mem_ctx,
+ name,
+ hash_size,
+ tdb_flags,
+ open_flags,
+ mode,
+ lock_order,
+ dbwrap_flags);
+ talloc_unlink(mem_ctx, lp_ctx);
return result;
}
--
2.13.6
From 6374fa5cbcaf6c64e47a8dcf753decdb90c7e6a1 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Sat, 18 Aug 2018 09:12:21 +0200
Subject: [PATCH 03/11] dbwrap_tdb: move a function call out of an if condition
At least for me this improves readability somewhat. No change in
behaviour.
Signed-off-by: Ralph Boehme <slow at samba.org>
---
lib/dbwrap/dbwrap_tdb.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/lib/dbwrap/dbwrap_tdb.c b/lib/dbwrap/dbwrap_tdb.c
index f59764535a2..19f2f009c0c 100644
--- a/lib/dbwrap/dbwrap_tdb.c
+++ b/lib/dbwrap/dbwrap_tdb.c
@@ -116,13 +116,16 @@ static struct db_record *db_tdb_fetch_locked_internal(
struct db_tdb_ctx *ctx = talloc_get_type_abort(db->private_data,
struct db_tdb_ctx);
struct tdb_fetch_locked_state state;
+ int ret;
state.mem_ctx = mem_ctx;
state.result = NULL;
- if ((tdb_parse_record(ctx->wtdb->tdb, key, db_tdb_fetchlock_parse,
- &state) < 0) &&
- (tdb_error(ctx->wtdb->tdb) != TDB_ERR_NOEXIST)) {
+ ret = tdb_parse_record(ctx->wtdb->tdb,
+ key,
+ db_tdb_fetchlock_parse,
+ &state);
+ if ((ret < 0) && (tdb_error(ctx->wtdb->tdb) != TDB_ERR_NOEXIST)) {
tdb_chainunlock(ctx->wtdb->tdb, key);
return NULL;
}
--
2.13.6
From 20d52e1cc3a1d1febc21895de69ca3f27311d9c8 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Sat, 18 Aug 2018 08:35:27 +0200
Subject: [PATCH 04/11] dbwrap_tdb: use struct initializer
This ensures all struct members are implicitly initialized.
Signed-off-by: Ralph Boehme <slow at samba.org>
---
lib/dbwrap/dbwrap_tdb.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/lib/dbwrap/dbwrap_tdb.c b/lib/dbwrap/dbwrap_tdb.c
index 19f2f009c0c..d695f3bd732 100644
--- a/lib/dbwrap/dbwrap_tdb.c
+++ b/lib/dbwrap/dbwrap_tdb.c
@@ -118,8 +118,9 @@ static struct db_record *db_tdb_fetch_locked_internal(
struct tdb_fetch_locked_state state;
int ret;
- state.mem_ctx = mem_ctx;
- state.result = NULL;
+ state = (struct tdb_fetch_locked_state) {
+ .mem_ctx = mem_ctx,
+ };
ret = tdb_parse_record(ctx->wtdb->tdb,
key,
--
2.13.6
From 72278a14494f943a8fdd43b73caa34d4a01b5df9 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Sun, 4 Mar 2018 23:39:05 +0100
Subject: [PATCH 05/11] dbwrap_ctdb: add error checking to ctdbd_dbpath()
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/lib/dbwrap/dbwrap_ctdb.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/source3/lib/dbwrap/dbwrap_ctdb.c b/source3/lib/dbwrap/dbwrap_ctdb.c
index 2885f4e7fda..999ca7224be 100644
--- a/source3/lib/dbwrap/dbwrap_ctdb.c
+++ b/source3/lib/dbwrap/dbwrap_ctdb.c
@@ -1856,6 +1856,10 @@ struct db_context *db_open_ctdb(TALLOC_CTX *mem_ctx,
db_path = ctdbd_dbpath(messaging_ctdb_connection(), db_ctdb,
db_ctdb->db_id);
+ if (db_path == NULL) {
+ DBG_ERR("ctdbd_dbpath failed\n");
+ return NULL;
+ }
result->persistent = persistent;
result->lock_order = lock_order;
--
2.13.6
From d142f0f4afc3be3e316200ae28ad26e357c3dcc9 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Tue, 4 Sep 2018 12:47:42 +0200
Subject: [PATCH 06/11] dbwrap_ctdb: simplify if condition
This just moves the talloc_memdup() out of the if condition as per
README.Coding.
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/lib/dbwrap/dbwrap_ctdb.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/source3/lib/dbwrap/dbwrap_ctdb.c b/source3/lib/dbwrap/dbwrap_ctdb.c
index 999ca7224be..6e5099d1351 100644
--- a/source3/lib/dbwrap/dbwrap_ctdb.c
+++ b/source3/lib/dbwrap/dbwrap_ctdb.c
@@ -1232,12 +1232,14 @@ static struct db_record *fetch_locked_internal(struct db_ctdb_ctx *ctx,
result->value.dsize = ctdb_data.dsize - sizeof(crec->header);
result->value.dptr = NULL;
- if ((result->value.dsize != 0)
- && !(result->value.dptr = (uint8_t *)talloc_memdup(
- result, ctdb_data.dptr + sizeof(crec->header),
- result->value.dsize))) {
- DEBUG(0, ("talloc failed\n"));
- TALLOC_FREE(result);
+ if (result->value.dsize != 0) {
+ result->value.dptr = talloc_memdup(
+ result, ctdb_data.dptr + sizeof(crec->header),
+ result->value.dsize);
+ if (result->value.dptr == NULL) {
+ DBG_ERR("talloc failed\n");
+ TALLOC_FREE(result);
+ }
}
SAFE_FREE(ctdb_data.dptr);
--
2.13.6
From 93af77cb3365a46a2df82b1b29043f346be999ab Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Sun, 9 Sep 2018 18:35:26 +0200
Subject: [PATCH 07/11] dbwrap_ctdb: README.Coding fixes in traverse_callback()
NULL initialize pointers, check function return values, explicit
variable check against NULL.
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/lib/dbwrap/dbwrap_ctdb.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/source3/lib/dbwrap/dbwrap_ctdb.c b/source3/lib/dbwrap/dbwrap_ctdb.c
index 6e5099d1351..fcae2adf56f 100644
--- a/source3/lib/dbwrap/dbwrap_ctdb.c
+++ b/source3/lib/dbwrap/dbwrap_ctdb.c
@@ -1532,11 +1532,18 @@ struct traverse_state {
static void traverse_callback(TDB_DATA key, TDB_DATA data, void *private_data)
{
struct traverse_state *state = (struct traverse_state *)private_data;
- struct db_record *rec;
- TALLOC_CTX *tmp_ctx = talloc_new(state->db);
+ struct db_record *rec = NULL;
+ TALLOC_CTX *tmp_ctx = NULL;
+
+ tmp_ctx = talloc_new(state->db);
+ if (tmp_ctx == NULL) {
+ DBG_ERR("talloc_new failed\n");
+ return;
+ }
+
/* we have to give them a locked record to prevent races */
rec = db_ctdb_fetch_locked(state->db, tmp_ctx, key);
- if (rec && rec->value.dsize > 0) {
+ if (rec != NULL && rec->value.dsize > 0) {
state->fn(rec, state->private_data);
}
talloc_free(tmp_ctx);
--
2.13.6
From b73879b0238c7ac587f87cc78d5bb9213f80d82d Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Sun, 9 Sep 2018 18:48:13 +0200
Subject: [PATCH 08/11] dbwrap_ctdb: use struct initializer in
db_ctdb_traverse_read()
This ensures all struct members are implicitly initialized.
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/lib/dbwrap/dbwrap_ctdb.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/source3/lib/dbwrap/dbwrap_ctdb.c b/source3/lib/dbwrap/dbwrap_ctdb.c
index fcae2adf56f..83218183b8b 100644
--- a/source3/lib/dbwrap/dbwrap_ctdb.c
+++ b/source3/lib/dbwrap/dbwrap_ctdb.c
@@ -1757,10 +1757,11 @@ static int db_ctdb_traverse_read(struct db_context *db,
struct db_ctdb_ctx);
struct traverse_state state;
- state.db = db;
- state.fn = fn;
- state.private_data = private_data;
- state.count = 0;
+ state = (struct traverse_state) {
+ .db = db,
+ .fn = fn,
+ .private_data = private_data,
+ };
if (db->persistent) {
/* for persistent databases we don't need to do a ctdb traverse,
--
2.13.6
From 82f5f2be2676f6bcd52434b7511b2cf4044815c9 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Sun, 9 Sep 2018 18:50:14 +0200
Subject: [PATCH 09/11] dbwrap_ctdb: use struct initializer in
db_ctdb_traverse()
This ensures all struct members are implicitly initialized.
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/lib/dbwrap/dbwrap_ctdb.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/source3/lib/dbwrap/dbwrap_ctdb.c b/source3/lib/dbwrap/dbwrap_ctdb.c
index 83218183b8b..6c51be5d5c2 100644
--- a/source3/lib/dbwrap/dbwrap_ctdb.c
+++ b/source3/lib/dbwrap/dbwrap_ctdb.c
@@ -1625,10 +1625,11 @@ static int db_ctdb_traverse(struct db_context *db,
struct db_ctdb_ctx);
struct traverse_state state;
- state.db = db;
- state.fn = fn;
- state.private_data = private_data;
- state.count = 0;
+ state = (struct traverse_state) {
+ .db = db,
+ .fn = fn,
+ .private_data = private_data,
+ };
if (db->persistent) {
struct tdb_context *ltdb = ctx->wtdb->tdb;
--
2.13.6
From 7489dd2639d409a54c774f9b17ef58c8d9148910 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Sun, 9 Sep 2018 18:51:43 +0200
Subject: [PATCH 10/11] dbwrap_ctdb: increment record count in
traverse_callback()
state->count wasn't incremented and is returned at the end of a
dbwrap_traverse().
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/lib/dbwrap/dbwrap_ctdb.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/source3/lib/dbwrap/dbwrap_ctdb.c b/source3/lib/dbwrap/dbwrap_ctdb.c
index 6c51be5d5c2..fdd45aaa7ab 100644
--- a/source3/lib/dbwrap/dbwrap_ctdb.c
+++ b/source3/lib/dbwrap/dbwrap_ctdb.c
@@ -1545,6 +1545,7 @@ static void traverse_callback(TDB_DATA key, TDB_DATA data, void *private_data)
rec = db_ctdb_fetch_locked(state->db, tmp_ctx, key);
if (rec != NULL && rec->value.dsize > 0) {
state->fn(rec, state->private_data);
+ state->count++;
}
talloc_free(tmp_ctx);
}
--
2.13.6
From a46a42233c02eac3a8f0de9cb941f332a790a010 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Mon, 10 Sep 2018 12:50:01 +0200
Subject: [PATCH 11/11] dbwrap_ctdb: return correct record count for a
persistent db read-only traverse
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/lib/dbwrap/dbwrap_ctdb.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/source3/lib/dbwrap/dbwrap_ctdb.c b/source3/lib/dbwrap/dbwrap_ctdb.c
index fdd45aaa7ab..956cc2f94fc 100644
--- a/source3/lib/dbwrap/dbwrap_ctdb.c
+++ b/source3/lib/dbwrap/dbwrap_ctdb.c
@@ -1768,7 +1768,15 @@ static int db_ctdb_traverse_read(struct db_context *db,
if (db->persistent) {
/* for persistent databases we don't need to do a ctdb traverse,
we can do a faster local traverse */
- return tdb_traverse_read(ctx->wtdb->tdb, traverse_persistent_callback_read, &state);
+ int nrecs;
+
+ nrecs = tdb_traverse_read(ctx->wtdb->tdb,
+ traverse_persistent_callback_read,
+ &state);
+ if (nrecs == -1) {
+ return -1;
+ }
+ return state.count;
}
ret = db_ctdbd_traverse(ctx->db_id, traverse_read_callback, &state);
--
2.13.6
More information about the samba-technical
mailing list