THINGS DONE:

0   Readded missing files i.e. regression tests.
    1
        svn add the new test (test_db_remove_subdb)
        Commit db->remove and the tokudb.bugs (bug 47 describes it)
    2
        Modified tests to use a test subdirectory for temp files,
        removed unneeded flags to DB_ENV->open
        test_db_dup.c
        test_db_already_exists.c
        test_db_close_no_open.c
        test_db_open_notexist_reopen.c
    3
        Added bug 48 (memory leak in DB->remove
        Need Valgrind to verify.
        svn add: test_db_remove_memleak.c


git-svn-id: file:///svn/tokudb@579 c7de825b-a66e-492c-adef-691d508d4ae1

newbrt/brt.c

@@ -1335,6 +1335,52 @@ int brt_open(BRT t, const char *fname, const char *dbname, int is_create, CACHET
     return 0;
 }
 
+int brt_remove_subdb(BRT brt, const char *dbname, u_int32_t flags) {
+    int r;
+    int r2 = 0;
+    int i;
+    int found = -1;
+
+    assert(flags == 0);
+    r = toku_read_and_pin_brt_header(brt->cf, &brt->h);
+    //TODO: What if r != 0? Is this possible?
+    //  We just called brt_open, so it should exist...
+    assert(r==0);  
+
+    assert(brt->h->unnamed_root==-1);
+    assert(brt->h->n_named_roots>=0);
+    for (i = 0; i < brt->h->n_named_roots; i++) {
+        if (strcmp(brt->h->names[i], dbname) == 0) {
+            found = i;
+            break;
+        }
+    }
+    if (found == -1) {
+        //Should not be possible.
+        r = ENOENT;
+        goto error;
+    }
+    //Free old db name
+    toku_free(brt->h->names[found]);
+    //TODO: Free Diskblocks including root
+    
+    for (i = found + 1; i < brt->h->n_named_roots; i++) {
+        brt->h->names[i - 1] = brt->h->names[i];
+        brt->h->roots[i - 1] = brt->h->roots[i];
+    }
+    brt->h->n_named_roots--;
+    brt->h->dirty = 1;
+    //TODO: What if n_named_roots becomes 0?  Should we handle it specially?  Should we delete the file?
+    if ((brt->h->names = toku_realloc(brt->h->names, (brt->h->n_named_roots)*sizeof(*brt->h->names))) == 0)   { assert(errno==ENOMEM); r=ENOMEM; goto error; }
+    if ((brt->h->roots = toku_realloc(brt->h->roots, (brt->h->n_named_roots)*sizeof(*brt->h->roots))) == 0)   { assert(errno==ENOMEM); r=ENOMEM; goto error; }
+
+error:
+    r2 = toku_unpin_brt_header(brt);
+    assert(r2==0);//TODO: Can r2 be non 0?
+    assert(brt->h==0);
+    return r ? r : r2;
+}
+
 int open_brt (const char *fname, const char *dbname, int is_create, BRT *newbrt, int nodesize, CACHETABLE cachetable,
 	      int (*compare_fun)(DB*,const DBT*,const DBT*)) {
     BRT brt;

newbrt/brt.h

@@ -19,6 +19,7 @@ int brt_set_bt_compare(BRT, int (*bt_compare)(DB *, const DBT*, const DBT*));
 int brt_set_dup_compare(BRT, int (*dup_compare)(DB *, const DBT*, const DBT*));
 int brt_set_cachetable(BRT, CACHETABLE);
 int brt_open(BRT, const char *fname, const char *dbname, int is_create, CACHETABLE ct);
+int brt_remove_subdb(BRT brt, const char *dbname, u_int32_t flags);
 
 int brt_insert (BRT, DBT *, DBT *, DB*, TOKUTXN);
 int brt_lookup (BRT brt, DBT *k, DBT *v, DB*db);

src/tests/test_db_already_exists.c

@@ -1,8 +1,12 @@
+#include <stdlib.h>
+#include <sys/stat.h>
 #include <stdio.h>
 #include <assert.h>
 #include <unistd.h>
 #include <db.h>
 
+#define DIR "dir.test_db_already_exists"
+
 int main() {
     DB_ENV * const null_env = 0;
     DB *db;
@@ -10,7 +14,9 @@ int main() {
     const char * const fname = "test.already.exists.brt";
     int r;
 
-    r = unlink(fname);
+    system("rm -rf " DIR);
+    r=mkdir(DIR, 0777); assert(r==0);
+    r=chdir(DIR);       assert(r==0);
 
     r = db_create(&db, null_env, 0);
     assert(r == 0);
@@ -33,5 +39,7 @@ int main() {
     r = db->close(db, 0);
     assert(r == 0);
 
+    r=chdir("..");       assert(r==0);
+
     return 0;
 }

src/tests/test_db_close_no_open.c

@@ -1,4 +1,4 @@
-/* Simple test of logging.  Can I start a TokuDB with logging enabled? */
+/* Can I close a db without opening it? */
 #include <assert.h>
 #include <stdlib.h>
 #include <sys/stat.h>
@@ -15,7 +15,7 @@ int main (int argc, char *argv[]) {
     system("rm -rf " DIR);
     r=mkdir(DIR, 0777);       assert(r==0);
     r=db_env_create(&env, 0); assert(r==0);
-    r=env->open(env, DIR, DB_INIT_LOCK|DB_INIT_LOG|DB_INIT_MPOOL|DB_INIT_TXN|DB_PRIVATE|DB_CREATE, 0777); assert(r==0);
+    r=env->open(env, DIR, DB_PRIVATE|DB_CREATE, 0777); assert(r==0);
     r=db_create(&db, env, 0); assert(r==0);
     r=db->close(db, 0);       assert(r==0);
     r=env->close(env, 0);     assert(r==0);

src/tests/test_db_dup.c

@@ -1,8 +1,12 @@
+#include <stdlib.h>
+#include <sys/stat.h>
 #include <stdio.h>
 #include <assert.h>
 #include <unistd.h>
 #include <db.h>
 
+#define DIR "dir.test_db_dup"
+
 int main() {
     DB_ENV * const null_env = 0;
     DB *db;
@@ -10,7 +14,9 @@ int main() {
     const char * const fname = "test.dup.brt";
     int r;
 
-    unlink(fname);
+    system("rm -rf " DIR);
+    r=mkdir(DIR, 0777); assert(r==0);
+    r=chdir(DIR);       assert(r==0);
 
     /* create the dup database file */
     r = db_create(&db, null_env, 0);
@@ -51,5 +57,7 @@ int main() {
     r = db->close(db, 0);
     assert(r == 0);
 
+    r=chdir("..");       assert(r==0);
+
     return 0;
 }

src/tests/test_db_open_notexist_reopen.c

@@ -14,7 +14,7 @@ int main (int argc, char *argv[]) {
     system("rm -rf " DIR);
     r=mkdir(DIR, 0777);       assert(r==0);
     r=db_env_create(&env, 0); assert(r==0);
-    r=env->open(env, DIR, DB_INIT_LOCK|DB_INIT_LOG|DB_INIT_MPOOL|DB_INIT_TXN|DB_PRIVATE|DB_CREATE, 0777); assert(r==0);
+    r=env->open(env, DIR, DB_PRIVATE|DB_CREATE, 0777); assert(r==0);
     r=db_create(&db, env, 0); assert(r==0);
     r=db->open(db, NULL, "doesnotexist.db", "testdb", DB_BTREE, 0, 0666); assert(r==ENOENT);
     r=db->open(db, NULL, "doesnotexist.db", "testdb", DB_BTREE, DB_CREATE, 0666); assert(r==0);

src/tests/test_db_remove_memleak.c

@@ -0,0 +1,39 @@
+/* Does removing a database free the DB structure's memory? */
+#include <assert.h>
+#include <stdlib.h>
+#include <sys/stat.h>
+#include <errno.h>
+#include <db.h>
+#include <string.h>
+
+#define DIR "dir.test_db_remove_memleak"
+DB_ENV *env;
+DB *db;
+DBT key;
+DBT data;
+
+int main (int argc, char *argv[]) {
+    int r;
+    system("rm -rf " DIR);
+    r=mkdir(DIR, 0777);         assert(r==0);
+    memset(&key, 0, sizeof(key));
+    memset(&data, 0, sizeof(data));
+    key.size = sizeof("name");
+    key.data = "name";
+    
+    r=db_env_create(&env, 0);   assert(r==0);
+    r=env->open(env, DIR, DB_PRIVATE|DB_CREATE, 0777); assert(r==0);
+
+    r=db_create(&db, env, 0);   assert(r==0);
+    r=db->open(db, NULL, "master.db", NULL, DB_BTREE, DB_CREATE, 0666); assert(r==0);
+    data.size = sizeof("first.db");
+    data.data = "first.db";
+    db->put(db, NULL, &key, &data, 0);
+    r=db->close(db, 0);         assert(r==0);
+
+    r=db_create(&db, env, 0);   assert(r==0);
+    r=db->remove(db, "master.db", NULL, 0); assert(r==0);
+
+    r=env->close(env, 0);     assert(r==0);
+    return 0;
+}

src/tests/test_db_remove_subdb.c

@@ -0,0 +1,89 @@
+/* Does removing subdatabases corrupt the db file/other dbs in that file? (when nothing else open) */
+#include <assert.h>
+#include <stdlib.h>
+#include <sys/stat.h>
+#include <errno.h>
+#include <db.h>
+#include <string.h>
+
+#define DIR "dir.test_db_remove_subdb"
+DB_ENV *env;
+DB *db;
+DBT key;
+DBT data;
+
+int main (int argc, char *argv[]) {
+    int r;
+    system("rm -rf " DIR);
+    r=mkdir(DIR, 0777);         assert(r==0);
+    memset(&key, 0, sizeof(key));
+    memset(&data, 0, sizeof(data));
+    key.size = sizeof("name");
+    key.data = "name";
+    
+    r=db_env_create(&env, 0);   assert(r==0);
+    r=env->open(env, DIR, DB_PRIVATE|DB_CREATE, 0777); assert(r==0);
+
+    r=db_create(&db, env, 0);   assert(r==0);
+    r=db->open(db, NULL, "master.db", "first", DB_BTREE, DB_CREATE, 0666); assert(r==0);
+    data.size = sizeof("first.db");
+    data.data = "first.db";
+    db->put(db, NULL, &key, &data, 0);
+    r=db->close(db, 0);         assert(r==0);
+
+    r=db_create(&db, env, 0);   assert(r==0);
+    r=db->remove(db, "master.db", "second", 0); assert(r==ENOENT);
+
+    r=db_create(&db, env, 0);   assert(r==0);
+    r=db->open(db, NULL, "master.db", "second", DB_BTREE, DB_CREATE, 0666); assert(r==0);
+    key.size = sizeof("name");
+    key.data = "name";
+    data.size = sizeof("second.db");
+    data.data = "second.db";
+    db->put(db, NULL, &key, &data, 0);
+    r=db->close(db, 0);         assert(r==0);
+
+    r=db_create(&db, env, 0);   assert(r==0);
+    r=db->open(db, NULL, "master.db", "third", DB_BTREE, DB_CREATE, 0666); assert(r==0);
+    key.size = sizeof("name");
+    key.data = "name";
+    data.size = sizeof("third.db");
+    data.data = "third.db";
+    db->put(db, NULL, &key, &data, 0);
+    r=db->close(db, 0);         assert(r==0);
+
+    r=db_create(&db, env, 0);   assert(r==0);
+    r=db->remove(db, "master.db", "second", 0); assert(r==0);
+
+    r=db_create(&db, env, 0);   assert(r==0);
+    r=db->remove(db, "master.db", "second", 0); assert(r==ENOENT);
+
+    memset(&key, 0, sizeof(key));
+    memset(&data, 0, sizeof(data));
+    key.size = sizeof("name");
+    key.data = "name";
+
+    //Verify data still exists in first/third
+    r=db_create(&db, env, 0);   assert(r==0);
+    r=db->open(db, NULL, "master.db", "first", DB_BTREE, 0, 0666); assert(r==0);
+    r=db->get(db, NULL, &key, &data, 0);    assert(r==0);
+    assert(!strcmp(data.data, "first.db"));
+    r=db->close(db, 0);         assert(r==0);
+    
+    r=db_create(&db, env, 0);   assert(r==0);
+    r=db->open(db, NULL, "master.db", "third", DB_BTREE, 0, 0666); assert(r==0);
+    r=db->get(db, NULL, &key, &data, 0);    assert(r==0);
+    assert(!strcmp(data.data, "third.db"));
+    r=db->close(db, 0);         assert(r==0);
+    
+    //Verify second is gone.
+    r=db_create(&db, env, 0);   assert(r==0);
+    r=db->open(db, NULL, "master.db", "second", DB_BTREE, 0, 0666); assert(r==ENOENT);
+    //Create again, verify it does not have its old data.
+    r=db->open(db, NULL, "master.db", "second", DB_BTREE, DB_CREATE, 0666); assert(r==0);
+    r=db->get(db, NULL, &key, &data, 0);    assert(r==DB_NOTFOUND);
+    
+    r=db->close(db, 0);       assert(r==0);
+    r=env->close(env, 0);     assert(r==0);
+    return 0;
+}

src/ydb.c

@@ -24,6 +24,8 @@ struct __toku_db_txn_internal {
     DB_TXN *parent;
 };
 
+static char *construct_full_name(const char *dir, const char *fname);
+    
 void __toku_db_env_err(const DB_ENV * env __attribute__ ((__unused__)), int error, const char *fmt, ...) {
     va_list ap;
     va_start(ap, fmt);
@@ -104,7 +106,12 @@ int __toku_db_env_open(DB_ENV * env, const char *home, u_int32_t flags, int mode
     env->i->dir = strdup(home);
     if (env->i->dir == 0) 
         return ENOMEM;
-
+    if (0) {
+        died1:
+        toku_free(env->i->dir);
+        env->i->dir = NULL;
+        return r;
+    }
     env->i->open_flags = flags;
     env->i->open_mode = mode;
 
@@ -391,6 +398,26 @@ int __toku_db_key_range(DB * db, DB_TXN * txn, DBT * dbt, DB_KEY_RANGE * kr, u_i
     abort();
 }
 
+static int construct_full_name_in_buf(const char *dir, const char *fname, char* full, int length) {
+    int l;
+
+    if (!full) return EINVAL;
+    l = snprintf(full, length, "%s", dir);
+    if (l >= length) return ENAMETOOLONG;
+    if (l == 0 || full[l - 1] != '/') {
+        if (l + 1 == length) return ENAMETOOLONG;
+            
+        /* Didn't put a slash down. */
+        if (fname[0] != '/') {
+            full[l++] = '/';
+            full[l] = 0;
+        }
+    }
+    l += snprintf(full + l, length - l, "%s", fname);
+    if (l >= length) return ENAMETOOLONG;
+    return 0;
+}
+
 static char *construct_full_name(const char *dir, const char *fname) {
     if (fname[0] == '/')
         dir = "";
@@ -399,18 +426,10 @@ static char *construct_full_name(const char *dir, const char *fname) {
         int fnamelen = strlen(fname);
         int len = dirlen + fnamelen + 2;        // One for the / between (which may not be there).  One for the trailing null.
         char *result = toku_malloc(len);
-        if (result) {
+        // printf("%s:%d len(%d)=%d+%d+2\n", __FILE__, __LINE__, len, dirlen, fnamelen);
-            int l;
+        if (construct_full_name_in_buf(dir, fname, result, len) != 0) {
-            // printf("%s:%d len(%d)=%d+%d+2\n", __FILE__, __LINE__, len, dirlen, fnamelen);
+            toku_free(result);
-            l = snprintf(result, len, "%s", dir);
+            result = NULL;
-            if (l == 0 || result[l - 1] != '/') {
-                /* Didn't put a slash down. */
-                if (fname[0] != '/') {
-                    result[l++] = '/';
-                    result[l] = 0;
-                }
-            }
-            l += snprintf(result + l, len - l, "%s", fname);
         }
         return result;
     }
@@ -493,11 +512,30 @@ int __toku_db_put(DB * db, DB_TXN * txn, DBT * key, DBT * data, u_int32_t flags)
 
 int __toku_db_remove(DB * db, const char *fname, const char *dbname, u_int32_t flags) {
     int r;
+    int r2;
     char ffull[PATH_MAX];
-    assert(dbname == 0);
+
-    r = snprintf(ffull, PATH_MAX, "%s%s", db->dbenv->i->dir, fname);
+    //TODO: Verify DB* db not yet opened
-    assert(r < PATH_MAX);
+
-    return unlink(ffull);
+    if (dbname) {
+        //TODO: Verify the target db is not open
+        //TODO: Use master database (instead of manual edit) when implemented.
+
+        if ((r = db->open(db, NULL, fname, dbname, DB_BTREE, 0, 0777)) != 0) goto cleanup;
+        r = brt_remove_subdb(db->i->brt, dbname, flags);
+cleanup:
+        r2 = db->close(db, 0);
+        return r ? r : r2;
+    }
+    //TODO: Verify db file not in use. (all dbs in the file must be unused)
+    if ((r = construct_full_name_in_buf(db->dbenv->i->dir, fname, ffull, sizeof(ffull))) != 0) {
+        //Name too long.
+        assert(r == ENAMETOOLONG);
+        return r;
+    }
+    r2 = db->close(db, 0);
+    if (r == 0 && r2 == 0) r = unlink(ffull);
+    return r ? r : r2;
 }
 
 int __toku_db_rename(DB * db, const char *namea, const char *nameb, const char *namec, u_int32_t flags) {