tdb: new test, cleanup old tests by centralizing lock tracking.

diff --git a/ccan/tdb/test/external-transaction.c b/ccan/tdb/test/external-transaction.c
index 37dd50841eb14b66d00d8b01dd435a5eaaa9a681..0a0b4b87685f03803c4b66e01bd172e818c5086d 100644 (file)
--- a/ccan/tdb/test/external-transaction.c
+++ b/ccan/tdb/test/external-transaction.c
@@ -76,7 +76,11 @@ static int do_operation(enum operation op, const char *name)
        } else if (op == CHECK_KEEP_OPENED) {
                return tdb_check(tdb, NULL, 0) == 0;
        } else if (op == NEEDS_RECOVERY_KEEP_OPENED) {
        } else if (op == CHECK_KEEP_OPENED) {
                return tdb_check(tdb, NULL, 0) == 0;
        } else if (op == NEEDS_RECOVERY_KEEP_OPENED) {

+#if 0

                return tdb_maybe_needs_recovery(tdb);
                return tdb_maybe_needs_recovery(tdb);

+#else
+               return 0;
+#endif

        }
 
        alarmed = 0;
        }
 
        alarmed = 0;

diff --git a/ccan/tdb/test/lock-tracking.c b/ccan/tdb/test/lock-tracking.c
new file mode 100644 (file)
index 0000000..bc4be14
--- /dev/null
+++ b/ccan/tdb/test/lock-tracking.c
@@ -0,0 +1,103 @@
+/* We save the locks so we can reaquire them. */
+#include <unistd.h>
+#include <fcntl.h>
+#include <stdarg.h>
+#include <stdlib.h>
+#include <ccan/tap/tap.h>
+
+struct lock {
+       struct lock *next;
+       unsigned int off;
+       unsigned int len;
+       int type;
+};
+static struct lock *locks;
+int locking_errors = 0;
+void (*unlock_callback)(int fd);
+
+int fcntl_with_lockcheck(int fd, int cmd, ... /* arg */ )
+{
+       va_list ap;
+       int ret, arg3;
+       struct flock *fl;
+
+       if (cmd != F_SETLK && cmd != F_SETLKW) {
+               /* This may be totally bogus, but we don't know in general. */
+               va_start(ap, cmd);
+               arg3 = va_arg(ap, int);
+               va_end(ap);
+
+               return fcntl(fd, cmd, arg3);
+       }
+
+       va_start(ap, cmd);
+       fl = va_arg(ap, struct flock *);
+       va_end(ap);
+
+       if (fl->l_type == F_UNLCK) {
+               struct lock **l;
+               struct lock *old = NULL;
+                       
+               for (l = &locks; *l; l = &(*l)->next) {
+                       if ((*l)->off == fl->l_start
+                           && (*l)->len == fl->l_len) {
+                               old = *l;
+                               *l = (*l)->next;
+                               free(old);
+                               break;
+                       }
+               }
+               if (!old) {
+                       diag("Unknown unlock %u@%u",
+                            (int)fl->l_len, (int)fl->l_start);
+                       locking_errors++;
+               }
+       } else {
+               struct lock *new, *i;
+               unsigned int fl_end = fl->l_start + fl->l_len;
+               if (fl->l_len == 0)
+                       fl_end = (unsigned int)-1;
+
+               /* Check for overlaps: we shouldn't do this. */
+               for (i = locks; i; i = i->next) {
+                       unsigned int i_end = i->off + i->len;
+                       if (i->len == 0)
+                               i_end = (unsigned int)-1;
+
+                       if (fl->l_start >= i->off && fl->l_start < i_end)
+                               break;
+                       if (fl_end >= i->off && fl_end < i_end)
+                               break;
+               }
+               if (i) {
+                       diag("%s lock %u@%u overlaps %u@%u",
+                            fl->l_type == F_WRLCK ? "write" : "read",
+                            (int)fl->l_len, (int)fl->l_start,
+                            i->len, (int)i->off);
+                       locking_errors++;
+               }
+               new = malloc(sizeof *new);
+               new->off = fl->l_start;
+               new->len = fl->l_len;
+               new->type = fl->l_type;
+               new->next = locks;
+               locks = new;
+       }
+
+       ret = fcntl(fd, cmd, fl);
+       if (ret == 0 && fl->l_type == F_UNLCK && unlock_callback)
+               unlock_callback(fd);
+       return ret;
+}
+
+int forget_locking(void)
+{
+       unsigned int num = 0;
+       while (locks) {
+               struct lock *next = locks->next;
+               free(locks);
+               locks = next;
+               num++;
+       }
+       return num;
+}

diff --git a/ccan/tdb/test/lock-tracking.h b/ccan/tdb/test/lock-tracking.h
new file mode 100644 (file)
index 0000000..9afdd09
--- /dev/null
+++ b/ccan/tdb/test/lock-tracking.h
@@ -0,0 +1,14 @@
+#ifndef LOCK_TRACKING_H
+#define LOCK_TRACKING_H
+/* Set this if you want a callback after fnctl unlock. */
+extern void (*unlock_callback)(int fd);
+
+/* Replacement fcntl. */
+int fcntl_with_lockcheck(int fd, int cmd, ... /* arg */ );
+
+/* Discard locking info: returns number of locks outstanding. */
+unsigned int forget_locking(void);
+
+/* Number of errors in locking. */
+extern int locking_errors;
+#endif /* LOCK_TRACKING_H */

diff --git a/ccan/tdb/test/run-die-during-transaction.c b/ccan/tdb/test/run-die-during-transaction.c
index 5f9d7f9f92d8600d69a3537bb11642e2777178c4..d80527f51b0e2147100a7bc12df22ea6bfca12b5 100644 (file)
--- a/ccan/tdb/test/run-die-during-transaction.c
+++ b/ccan/tdb/test/run-die-during-transaction.c
@@ -1,13 +1,13 @@
 #define _XOPEN_SOURCE 500
 #include <unistd.h>
 #define _XOPEN_SOURCE 500
 #include <unistd.h>

+#include "lock-tracking.h"

 static ssize_t pwrite_check(int fd, const void *buf, size_t count, off_t offset);
 static ssize_t write_check(int fd, const void *buf, size_t count);
 static ssize_t pwrite_check(int fd, const void *buf, size_t count, off_t offset);
 static ssize_t write_check(int fd, const void *buf, size_t count);

-static int fcntl_check(int fd, int cmd, ... /* arg */ );

 static int ftruncate_check(int fd, off_t length);
 
 #define pwrite pwrite_check
 #define write write_check
 static int ftruncate_check(int fd, off_t length);
 
 #define pwrite pwrite_check
 #define write write_check

-#define fcntl fcntl_check
+#define fcntl fcntl_with_lockcheck

 #define ftruncate ftruncate_check
 
 #include <ccan/tdb/tdb.h>
 #define ftruncate ftruncate_check
 
 #include <ccan/tdb/tdb.h>


@@ -39,15 +39,6 @@ static int target, current;
 static jmp_buf jmpbuf;
 #define TEST_DBNAME "/tmp/test7.tdb"
 
 static jmp_buf jmpbuf;
 #define TEST_DBNAME "/tmp/test7.tdb"
 

-/* We save the locks so we can reaquire them. */
-struct lock {
-       struct lock *next;
-       off_t off;
-       unsigned int len;
-       int type;
-};
-static struct lock *locks;
-

 static void taplog(struct tdb_context *tdb,
                   enum tdb_debug_level level,
                   const char *fmt, ...)
 static void taplog(struct tdb_context *tdb,
                   enum tdb_debug_level level,
                   const char *fmt, ...)


@@ -65,9 +56,9 @@ static void taplog(struct tdb_context *tdb,
        diag("%s", line);
 }
 
        diag("%s", line);
 }
 

-static void check_file_contents(int fd)
+static void maybe_die(int fd)

 {
 {

-       if (current++ == target) {
+       if (in_transaction && current++ == target) {

                longjmp(jmpbuf, 1);
        }
 }
                longjmp(jmpbuf, 1);
        }
 }


@@ -77,15 +68,13 @@ static ssize_t pwrite_check(int fd,
 {
        ssize_t ret;
 
 {
        ssize_t ret;
 

-       if (in_transaction)
-               check_file_contents(fd);
+       maybe_die(fd);

 
        ret = pwrite(fd, buf, count, offset);
        if (ret != count)
                return ret;
 
 
        ret = pwrite(fd, buf, count, offset);
        if (ret != count)
                return ret;
 

-       if (in_transaction)
-               check_file_contents(fd);
+       maybe_die(fd);

        return ret;
 }
 
        return ret;
 }
 


@@ -93,68 +82,13 @@ static ssize_t write_check(int fd, const void *buf, size_t count)
 {
        ssize_t ret;
 
 {
        ssize_t ret;
 

-       if (in_transaction)
-               check_file_contents(fd);
+       maybe_die(fd);

 
        ret = write(fd, buf, count);
        if (ret != count)
                return ret;
 
 
        ret = write(fd, buf, count);
        if (ret != count)
                return ret;
 

-       if (in_transaction)
-               check_file_contents(fd);
-       return ret;
-}
-
-extern int fcntl(int fd, int cmd, ... /* arg */ );
-
-static int fcntl_check(int fd, int cmd, ... /* arg */ )
-{
-       va_list ap;
-       int ret, arg3;
-       struct flock *fl;
-
-       if (cmd != F_SETLK && cmd != F_SETLKW) {
-               /* This may be totally bogus, but we don't know in general. */
-               va_start(ap, cmd);
-               arg3 = va_arg(ap, int);
-               va_end(ap);
-
-               return fcntl(fd, cmd, arg3);
-       }
-
-       va_start(ap, cmd);
-       fl = va_arg(ap, struct flock *);
-       va_end(ap);
-
-       ret = fcntl(fd, cmd, fl);
-       if (ret == 0) {
-               if (fl->l_type == F_UNLCK) {
-                       struct lock **l;
-                       struct lock *old = NULL;
-                       
-                       for (l = &locks; *l; l = &(*l)->next) {
-                               if ((*l)->off == fl->l_start
-                                   && (*l)->len == fl->l_len) {
-                                       old = *l;
-                                       *l = (*l)->next;
-                                       free(old);
-                                       break;
-                               }
-                       }
-                       if (!old)
-                               errx(1, "Unknown lock");
-               } else {
-                       struct lock *new = malloc(sizeof *new);
-                       new->off = fl->l_start;
-                       new->len = fl->l_len;
-                       new->type = fl->l_type;
-                       new->next = locks;
-                       locks = new;
-               }
-       }
-
-       if (in_transaction && fl->l_type == F_UNLCK)
-               check_file_contents(fd);
+       maybe_die(fd);

        return ret;
 }
 
        return ret;
 }
 


@@ -162,13 +96,11 @@ static int ftruncate_check(int fd, off_t length)
 {
        int ret;
 
 {
        int ret;
 

-       if (in_transaction)
-               check_file_contents(fd);
+       maybe_die(fd);

 
        ret = ftruncate(fd, length);
 
 
        ret = ftruncate(fd, length);
 

-       if (in_transaction)
-               check_file_contents(fd);
+       maybe_die(fd);

        return ret;
 }
 
        return ret;
 }
 


@@ -216,6 +148,7 @@ reset:
                suppress_logging = false;
                target++;
                current = 0;
                suppress_logging = false;
                target++;
                current = 0;

+               forget_locking();

                goto reset;
        }
 
                goto reset;
        }
 


@@ -248,6 +181,9 @@ reset:
        external_agent_operation(agent, CLOSE, "");
 
        ok1(needed_recovery);
        external_agent_operation(agent, CLOSE, "");
 
        ok1(needed_recovery);

+       ok1(locking_errors == 0);
+       ok1(forget_locking() == 0);
+       locking_errors = 0;

        return true;
 }
 
        return true;
 }
 


@@ -260,6 +196,8 @@ int main(int argc, char *argv[])
        int i;
 
        plan_tests(6);
        int i;
 
        plan_tests(6);

+       unlock_callback = maybe_die;
+

        agent = prepare_external_agent();
        if (!agent)
                err(1, "preparing agent");
        agent = prepare_external_agent();
        if (!agent)
                err(1, "preparing agent");

diff --git a/ccan/tdb/test/run-no-lock-during-traverse.c b/ccan/tdb/test/run-no-lock-during-traverse.c
new file mode 100644 (file)
index 0000000..1eec04b
--- /dev/null
+++ b/ccan/tdb/test/run-no-lock-during-traverse.c
@@ -0,0 +1,113 @@
+#define _XOPEN_SOURCE 500
+#include <unistd.h>
+#include "lock-tracking.h"
+
+#define fcntl fcntl_with_lockcheck
+
+#include <ccan/tdb/tdb.h>
+#include <ccan/tdb/io.c>
+#include <ccan/tdb/tdb.c>
+#include <ccan/tdb/lock.c>
+#include <ccan/tdb/freelist.c>
+#include <ccan/tdb/traverse.c>
+#include <ccan/tdb/transaction.c>
+#include <ccan/tdb/error.c>
+#include <ccan/tdb/open.c>
+#include <ccan/tdb/check.c>
+#include <ccan/tap/tap.h>
+#include <stdlib.h>
+#include <err.h>
+
+#undef fcntl
+
+#define NUM_ENTRIES 10
+
+static bool prepare_entries(struct tdb_context *tdb)
+{
+       unsigned int i;
+       TDB_DATA key, data;
+       
+       for (i = 0; i < NUM_ENTRIES; i++) {
+               key.dsize = sizeof(i);
+               key.dptr = (void *)&i;
+               data.dsize = strlen("world");
+               data.dptr = (void *)"world";
+
+               if (tdb_store(tdb, key, data, 0) != 0)
+                       return false;
+       }
+       return true;
+}
+
+static void delete_entries(struct tdb_context *tdb)
+{
+       unsigned int i;
+       TDB_DATA key;
+
+       for (i = 0; i < NUM_ENTRIES; i++) {
+               key.dsize = sizeof(i);
+               key.dptr = (void *)&i;
+
+               ok1(tdb_delete(tdb, key) == 0);
+       }
+}
+
+/* We don't know how many times this will run. */
+static int delete_other(struct tdb_context *tdb, TDB_DATA key, TDB_DATA data,
+                       void *private_data)
+{
+       unsigned int i;
+       memcpy(&i, key.dptr, 4);
+       i = (i + 1) % NUM_ENTRIES;
+       key.dptr = (void *)&i;
+       if (tdb_delete(tdb, key) != 0)
+               (*(int *)private_data)++;
+       return 0;
+}
+
+static int delete_self(struct tdb_context *tdb, TDB_DATA key, TDB_DATA data,
+                       void *private_data)
+{
+       ok1(tdb_delete(tdb, key) == 0);
+       return 0;
+}
+
+int main(int argc, char *argv[])
+{
+       struct tdb_context *tdb;
+       int errors = 0;
+
+       plan_tests(41);
+       tdb = tdb_open("/tmp/test8.tdb", 1024, TDB_CLEAR_IF_FIRST,
+                      O_CREAT|O_TRUNC|O_RDWR, 0600);
+
+       ok1(tdb);
+       ok1(prepare_entries(tdb));
+       ok1(locking_errors == 0);
+       ok1(tdb_lockall(tdb) == 0);
+       ok1(locking_errors == 0);
+       tdb_traverse(tdb, delete_other, &errors);
+       ok1(errors == 0);
+       ok1(locking_errors == 0);
+       ok1(tdb_unlockall(tdb) == 0);
+
+       ok1(prepare_entries(tdb));
+       ok1(locking_errors == 0);
+       ok1(tdb_lockall(tdb) == 0);
+       ok1(locking_errors == 0);
+       tdb_traverse(tdb, delete_self, NULL);
+       ok1(locking_errors == 0);
+       ok1(tdb_unlockall(tdb) == 0);
+
+       ok1(prepare_entries(tdb));
+       ok1(locking_errors == 0);
+       ok1(tdb_lockall(tdb) == 0);
+       ok1(locking_errors == 0);
+       delete_entries(tdb);
+       ok1(locking_errors == 0);
+       ok1(tdb_unlockall(tdb) == 0);
+
+       ok1(tdb_close(tdb) == 0);       
+
+       return exit_status();
+}

diff --git a/ccan/tdb/test/run-open-during-transaction.c b/ccan/tdb/test/run-open-during-transaction.c
index 27356e785f6b00c59539887b5e82ae1a641c77a6..908dc7da6c44309560793b4abecccff1113af316 100644 (file)
--- a/ccan/tdb/test/run-open-during-transaction.c
+++ b/ccan/tdb/test/run-open-during-transaction.c
@@ -1,13 +1,14 @@
 #define _XOPEN_SOURCE 500
 #include <unistd.h>
 #define _XOPEN_SOURCE 500
 #include <unistd.h>

+#include "lock-tracking.h"
+

 static ssize_t pwrite_check(int fd, const void *buf, size_t count, off_t offset);
 static ssize_t write_check(int fd, const void *buf, size_t count);
 static ssize_t pwrite_check(int fd, const void *buf, size_t count, off_t offset);
 static ssize_t write_check(int fd, const void *buf, size_t count);

-static int fcntl_check(int fd, int cmd, ... /* arg */ );

 static int ftruncate_check(int fd, off_t length);
 
 #define pwrite pwrite_check
 #define write write_check
 static int ftruncate_check(int fd, off_t length);
 
 #define pwrite pwrite_check
 #define write write_check

-#define fcntl fcntl_check
+#define fcntl fcntl_with_lockcheck

 #define ftruncate ftruncate_check
 
 #include <ccan/tdb/tdb.h>
 #define ftruncate ftruncate_check
 
 #include <ccan/tdb/tdb.h>


@@ -107,6 +108,9 @@ static void check_for_agent(int fd, bool block)
 
 static void check_file_contents(int fd)
 {
 
 static void check_file_contents(int fd)
 {

+       if (!in_transaction)
+               return;
+

        if (agent_pending)
                check_for_agent(fd, false);
 
        if (agent_pending)
                check_for_agent(fd, false);
 


@@ -132,16 +136,14 @@ static ssize_t pwrite_check(int fd,
 {
        ssize_t ret;
 
 {
        ssize_t ret;
 

-       if (in_transaction)
-               check_file_contents(fd);
+       check_file_contents(fd);

 
        snapshot_uptodate = false;
        ret = pwrite(fd, buf, count, offset);
        if (ret != count)
                return ret;
 
 
        snapshot_uptodate = false;
        ret = pwrite(fd, buf, count, offset);
        if (ret != count)
                return ret;
 

-       if (in_transaction)
-               check_file_contents(fd);
+       check_file_contents(fd);

        return ret;
 }
 
        return ret;
 }
 


@@ -149,8 +151,7 @@ static ssize_t write_check(int fd, const void *buf, size_t count)
 {
        ssize_t ret;
 
 {
        ssize_t ret;
 

-       if (in_transaction)
-               check_file_contents(fd);
+       check_file_contents(fd);

 
        snapshot_uptodate = false;
 
 
        snapshot_uptodate = false;
 


@@ -158,37 +159,7 @@ static ssize_t write_check(int fd, const void *buf, size_t count)
        if (ret != count)
                return ret;
 
        if (ret != count)
                return ret;
 

-       if (in_transaction)
-               check_file_contents(fd);
-       return ret;
-}
-
-/* This seems to be a macro for glibc... */
-extern int fcntl(int fd, int cmd, ... /* arg */ );
-
-static int fcntl_check(int fd, int cmd, ... /* arg */ )
-{
-       va_list ap;
-       int ret, arg3;
-       struct flock *fl;
-
-       if (cmd != F_SETLK && cmd != F_SETLKW) {
-               /* This may be totally bogus, but we don't know in general. */
-               va_start(ap, cmd);
-               arg3 = va_arg(ap, int);
-               va_end(ap);
-
-               return fcntl(fd, cmd, arg3);
-       }
-
-       va_start(ap, cmd);
-       fl = va_arg(ap, struct flock *);
-       va_end(ap);
-
-       ret = fcntl(fd, cmd, fl);
-
-       if (in_transaction && fl->l_type == F_UNLCK)
-               check_file_contents(fd);
+       check_file_contents(fd);

        return ret;
 }
 
        return ret;
 }
 


@@ -196,15 +167,13 @@ static int ftruncate_check(int fd, off_t length)
 {
        int ret;
 
 {
        int ret;
 

-       if (in_transaction)
-               check_file_contents(fd);
+       check_file_contents(fd);

 
        snapshot_uptodate = false;
 
        ret = ftruncate(fd, length);
 
 
        snapshot_uptodate = false;
 
        ret = ftruncate(fd, length);
 

-       if (in_transaction)
-               check_file_contents(fd);
+       check_file_contents(fd);

        return ret;
 }
 
        return ret;
 }
 


@@ -220,6 +189,7 @@ int main(int argc, char *argv[])
        TDB_DATA key, data;
 
        plan_tests(20);
        TDB_DATA key, data;
 
        plan_tests(20);

+       unlock_callback = check_file_contents;

        agent = prepare_external_agent();
        if (!agent)
                err(1, "preparing agent");
        agent = prepare_external_agent();
        if (!agent)
                err(1, "preparing agent");