diff options
Diffstat (limited to 'lib')
| -rw-r--r-- | lib/ntdb/doc/TDB_porting.txt | 91 | ||||
| -rw-r--r-- | lib/ntdb/lock.c | 11 | ||||
| -rw-r--r-- | lib/ntdb/ntdb.c | 13 | ||||
| -rw-r--r-- | lib/ntdb/ntdb.h | 9 | ||||
| -rw-r--r-- | lib/ntdb/test/api-95-read-only-during-parse.c | 94 | ||||
| -rw-r--r-- | lib/ntdb/wscript | 1 |
6 files changed, 195 insertions, 24 deletions
diff --git a/lib/ntdb/doc/TDB_porting.txt b/lib/ntdb/doc/TDB_porting.txt index 8b0ca2fec8..5daf94b74b 100644 --- a/lib/ntdb/doc/TDB_porting.txt +++ b/lib/ntdb/doc/TDB_porting.txt @@ -170,23 +170,6 @@ Interface differences between TDB and NTDB. O_CREAT|O_RDWR, 0600, &hashsize); } -- ntdb does locking on read-only databases (ie. O_RDONLY passed to ntdb_open). - tdb did not: use the NTDB_NOLOCK flag if you want to suppress locking. - - Example: - #include <tdb.h> - #include <ntdb.h> - - struct tdb_context *tdb_example(void) - { - return tdb_open("example.tdb", 0, TDB_DEFAULT, O_RDONLY, 0); - } - - struct ntdb_context *ntdb_example(void) - { - return ntdb_open("example.ntdb", NTDB_NOLOCK, O_RDONLY, NULL); - } - - ntdb's log function is simpler than tdb's log function. The string is already formatted, is not terminated by a '\n', and it takes an enum ntdb_log_level not a tdb_debug_level, and which has only three @@ -304,6 +287,80 @@ Interface differences between TDB and NTDB. } } +- ntdb's ntdb_parse_record() takes a type-checked callback data + pointer, not a void * (though a void * pointer still works). The + callback function is allowed to do read operations on the database, + or write operations if you first call ntdb_lockall(). TDB's + tdb_parse_record() did not allow any database access within the + callback, could crash if you tried. + + Example: + #include <tdb.h> + #include <ntdb.h> + + static int tdb_parser(TDB_DATA key, TDB_DATA data, void *private_data) + { + TDB_DATA *expect = private_data; + + return data.dsize == expect->dsize + && !memcmp(data.dptr, expect->dptr, data.dsize); + } + + void tdb_example(struct tdb_context *tdb, TDB_DATA key, NTDB_DATA d) + { + switch (tdb_parse_record(tdb, key, tdb_parser, &d)) { + case -1: + printf("parse failed: %s\n", tdb_errorstr(tdb)); + break; + case 0: + printf("data was different!\n"); + break; + case 1: + printf("data was same!\n"); + break; + } + } + + static int ntdb_parser(TDB_DATA key, TDB_DATA data, TDB_DATA *expect) + { + return ntdb_deq(data, *expect); + } + + void ntdb_example(struct ntdb_context *ntdb, NTDB_DATA key, NTDB_DATA d) + { + enum NTDB_ERROR e; + + e = tdb_parse_record(tdb, key, tdb_parser, &d); + switch (e) { + case 0: + printf("data was different!\n"); + break; + case 1: + printf("data was same!\n"); + break; + default: + printf("parse failed: %s\n", ntdb_errorstr(e)); + break; + } + } + +- ntdb does locking on read-only databases (ie. O_RDONLY passed to ntdb_open). + tdb did not: use the NTDB_NOLOCK flag if you want to suppress locking. + + Example: + #include <tdb.h> + #include <ntdb.h> + + struct tdb_context *tdb_example(void) + { + return tdb_open("example.tdb", 0, TDB_DEFAULT, O_RDONLY, 0); + } + + struct ntdb_context *ntdb_example(void) + { + return ntdb_open("example.ntdb", NTDB_NOLOCK, O_RDONLY, NULL); + } + - Failure inside a transaction (such as a lock function failing) does not implicitly cancel the transaction; you still need to call ntdb_transaction_cancel(). diff --git a/lib/ntdb/lock.c b/lib/ntdb/lock.c index f6a811a3fa..4517e25568 100644 --- a/lib/ntdb/lock.c +++ b/lib/ntdb/lock.c @@ -188,15 +188,15 @@ static enum NTDB_ERROR ntdb_brlock(struct ntdb_context *ntdb, { int ret; - if (ntdb->flags & NTDB_NOLOCK) { - return NTDB_SUCCESS; - } - if (rw_type == F_WRLCK && (ntdb->flags & NTDB_RDONLY)) { return ntdb_logerr(ntdb, NTDB_ERR_RDONLY, NTDB_LOG_USE_ERROR, "Write lock attempted on read-only database"); } + if (ntdb->flags & NTDB_NOLOCK) { + return NTDB_SUCCESS; + } + /* A 32 bit system cannot open a 64-bit file, but it could have * expanded since then: check here. */ if ((size_t)(offset + len) != offset + len) { @@ -533,8 +533,9 @@ enum NTDB_ERROR ntdb_allrecord_lock(struct ntdb_context *ntdb, int ltype, enum NTDB_ERROR ecode; ntdb_bool_err berr; - if (ntdb->flags & NTDB_NOLOCK) + if (ntdb->flags & NTDB_NOLOCK) { return NTDB_SUCCESS; + } if (!check_lock_pid(ntdb, "ntdb_allrecord_lock", true)) { return NTDB_ERR_LOCK; diff --git a/lib/ntdb/ntdb.c b/lib/ntdb/ntdb.c index 6c75915899..5d56b33b5a 100644 --- a/lib/ntdb/ntdb.c +++ b/lib/ntdb/ntdb.c @@ -500,10 +500,23 @@ _PUBLIC_ enum NTDB_ERROR ntdb_parse_record_(struct ntdb_context *ntdb, if (!off) { ecode = NTDB_ERR_NOEXIST; } else { + unsigned int old_flags; NTDB_DATA d = ntdb_mkdata(keyp + key.dsize, rec_data_length(&rec)); + /* + * Make sure they don't try to write db, since they + * have read lock! They can if they've done + * ntdb_lockall(): if it was ntdb_lockall_read, that'll + * stop them doing a write operation anyway. + */ + old_flags = ntdb->flags; + if (!ntdb->file->allrecord_lock.count && + !(ntdb->flags & NTDB_NOLOCK)) { + ntdb->flags |= NTDB_RDONLY; + } ecode = parse(key, d, data); + ntdb->flags = old_flags; ntdb_access_release(ntdb, keyp); } diff --git a/lib/ntdb/ntdb.h b/lib/ntdb/ntdb.h index 8e8458e4d1..df3a9ddc4e 100644 --- a/lib/ntdb/ntdb.h +++ b/lib/ntdb/ntdb.h @@ -368,9 +368,14 @@ int64_t ntdb_traverse_(struct ntdb_context *ntdb, * * This avoids a copy for many cases, by handing you a pointer into * the memory-mapped database. It also locks the record to prevent - * other accesses at the same time. + * other accesses at the same time, so it won't change. * - * Do not alter the data handed to parse()! + * Within the @parse callback you can perform read operations on the + * database, but no write operations: no ntdb_store() or + * ntdb_delete(), for example. The exception is if you call + * ntdb_lockall() before ntdb_parse_record(). + * + * Never alter the data handed to parse()! */ #define ntdb_parse_record(ntdb, key, parse, data) \ ntdb_parse_record_((ntdb), (key), \ diff --git a/lib/ntdb/test/api-95-read-only-during-parse.c b/lib/ntdb/test/api-95-read-only-during-parse.c new file mode 100644 index 0000000000..8252b812d2 --- /dev/null +++ b/lib/ntdb/test/api-95-read-only-during-parse.c @@ -0,0 +1,94 @@ +/* Make sure write operations fail during ntdb_parse(). */ +#include "config.h" +#include "ntdb.h" +#include "tap-interface.h" +#include <sys/types.h> +#include <sys/stat.h> +#include <fcntl.h> +#include "logging.h" + +static struct ntdb_context *ntdb; + +/* We could get either of these. */ +static bool xfail(enum NTDB_ERROR ecode) +{ + return ecode == NTDB_ERR_RDONLY || ecode == NTDB_ERR_LOCK; +} + +static enum NTDB_ERROR parse(NTDB_DATA key, NTDB_DATA data, + NTDB_DATA *expected) +{ + NTDB_DATA add = ntdb_mkdata("another", strlen("another")); + + if (!ntdb_deq(data, *expected)) { + return NTDB_ERR_EINVAL; + } + + /* These should all fail.*/ + if (!xfail(ntdb_store(ntdb, add, add, NTDB_INSERT))) { + return NTDB_ERR_EINVAL; + } + tap_log_messages--; + + if (!xfail(ntdb_append(ntdb, key, add))) { + return NTDB_ERR_EINVAL; + } + tap_log_messages--; + + if (!xfail(ntdb_delete(ntdb, key))) { + return NTDB_ERR_EINVAL; + } + tap_log_messages--; + + if (!xfail(ntdb_transaction_start(ntdb))) { + return NTDB_ERR_EINVAL; + } + tap_log_messages--; + + if (!xfail(ntdb_chainlock(ntdb, key))) { + return NTDB_ERR_EINVAL; + } + tap_log_messages--; + + if (!xfail(ntdb_lockall(ntdb))) { + return NTDB_ERR_EINVAL; + } + tap_log_messages--; + + if (!xfail(ntdb_wipe_all(ntdb))) { + return NTDB_ERR_EINVAL; + } + tap_log_messages--; + + if (!xfail(ntdb_repack(ntdb))) { + return NTDB_ERR_EINVAL; + } + tap_log_messages--; + + /* Access the record one more time. */ + if (!ntdb_deq(data, *expected)) { + return NTDB_ERR_EINVAL; + } + + return NTDB_SUCCESS; +} + +int main(int argc, char *argv[]) +{ + unsigned int i; + int flags[] = { NTDB_DEFAULT, NTDB_NOMMAP, NTDB_CONVERT }; + NTDB_DATA key = ntdb_mkdata("hello", 5), data = ntdb_mkdata("world", 5); + + plan_tests(sizeof(flags) / sizeof(flags[0]) * 2 + 1); + for (i = 0; i < sizeof(flags) / sizeof(flags[0]); i++) { + ntdb = ntdb_open("api-95-read-only-during-parse.ntdb", + flags[i]|MAYBE_NOSYNC, + O_RDWR|O_CREAT|O_TRUNC, 0600, &tap_log_attr); + ok1(ntdb_store(ntdb, key, data, NTDB_INSERT) == NTDB_SUCCESS); + ok1(ntdb_parse_record(ntdb, key, parse, &data) == NTDB_SUCCESS); + ntdb_close(ntdb); + } + + ok1(tap_log_messages == 0); + return exit_status(); +} diff --git a/lib/ntdb/wscript b/lib/ntdb/wscript index f034631058..e211a9fc0c 100644 --- a/lib/ntdb/wscript +++ b/lib/ntdb/wscript @@ -79,6 +79,7 @@ def configure(conf): 'test/api-91-get-stats.c', 'test/api-92-get-set-readonly.c', 'test/api-93-repack.c', + 'test/api-95-read-only-during-parse.c', 'test/api-add-remove-flags.c', 'test/api-check-callback.c', 'test/api-firstkey-nextkey.c', |