Repository: bmatsuo/lmdb-go
Pull request #48 (2016-02-12 11:18:54)

lmdb: fix return value of Env.ReaderList when passed invalid arguments
When Env.ReaderList is called with a nil function argument it
behaves as if mdb_env_reader_list() was called with a NULL function
argument, returning an error.  Calling ReaderList on an invalid Env
returns an error as well, but has done so for a long time.


Patch:
diff --git a/lmdb/env.go b/lmdb/env.go
index d4c1e75..dfb49c0 100644
--- a/lmdb/env.go
+++ b/lmdb/env.go
@@ -124,12 +124,15 @@ func (env *Env) FD() (uintptr, error) {
 func (env *Env) ReaderList(fn func(string) error) error {
 	ctx, done := newMsgFunc(fn)
 	defer done()
+	if fn == nil {
+		ctx = 0
+	}
 
 	ret := C.lmdbgo_mdb_reader_list(env._env, C.size_t(ctx))
 	if ret >= 0 {
 		return nil
 	}
-	if ret < 0 {
+	if ret < 0 && ctx != 0 {
 		err := ctx.get().err
 		if err != nil {
 			return err
diff --git a/lmdb/lmdbgo.c b/lmdb/lmdbgo.c
index 6b80dbf..a245992 100644
--- a/lmdb/lmdbgo.c
+++ b/lmdb/lmdbgo.c
@@ -18,7 +18,9 @@ int lmdbgo_mdb_msg_func_proxy(const char *msg, void *ctx) {
 int lmdbgo_mdb_reader_list(MDB_env *env, size_t ctx) {
     // list readers using a static proxy function that does dynamic dispatch on
     // ctx.
-    return mdb_reader_list(env, &lmdbgo_mdb_msg_func_proxy, (void *)ctx);
+	if (ctx)
+		return mdb_reader_list(env, &lmdbgo_mdb_msg_func_proxy, (void *)ctx);
+	return mdb_reader_list(env, 0, (void *)ctx);
 }
 
 int lmdbgo_mdb_del(MDB_txn *txn, MDB_dbi dbi, void *kdata, size_t kn, void *vdata, size_t vn) {
diff --git a/lmdb/msgfunc.go b/lmdb/msgfunc.go
index f5056fc..4f861e5 100644
--- a/lmdb/msgfunc.go
+++ b/lmdb/msgfunc.go
@@ -17,10 +17,6 @@ import (
 //export lmdbgoMDBMsgFuncBridge
 func lmdbgoMDBMsgFuncBridge(cmsg C.lmdbgo_ConstCString, _ctx C.size_t) C.int {
 	ctx := msgctx(_ctx).get()
-	if ctx.fn == nil {
-		return 0
-	}
-
 	msg := C.GoString(cmsg.p)
 	err := ctx.fn(msg)
 	if err != nil {
@@ -52,8 +48,12 @@ var msgctxn uint32
 var msgctxm = map[msgctx]*_msgctx{}
 var msgctxmlock sync.RWMutex
 
+func nextctx() msgctx {
+	return msgctx(atomic.AddUint32(&msgctxn, 1))
+}
+
 func newMsgFunc(fn msgfunc) (ctx msgctx, done func()) {
-	ctx = msgctx(atomic.AddUint32(&msgctxn, 1))
+	ctx = nextctx()
 	ctx.register(fn)
 	return ctx, ctx.deregister
 }
@@ -61,6 +61,7 @@ func newMsgFunc(fn msgfunc) (ctx msgctx, done func()) {
 func (ctx msgctx) register(fn msgfunc) {
 	msgctxmlock.Lock()
 	if _, ok := msgctxm[ctx]; ok {
+		msgctxmlock.Unlock()
 		panic("msgfunc conflict")
 	}
 	msgctxm[ctx] = &_msgctx{fn: fn}


Test patch:
diff --git a/lmdb/env_test.go b/lmdb/env_test.go
index dd5559b..194986a 100644
--- a/lmdb/env_test.go
+++ b/lmdb/env_test.go
@@ -344,6 +344,27 @@ func TestEnv_ReaderList_error(t *testing.T) {
 	}
 }
 
+func TestEnv_ReaderList_envInvalid(t *testing.T) {
+	err := (&Env{}).ReaderList(func(msg string) error {
+		t.Logf("%s", msg)
+		return nil
+	})
+	if err == nil {
+		t.Errorf("expected error")
+	}
+}
+
+func TestEnv_ReaderList_nilFunc(t *testing.T) {
+	env, err := NewEnv()
+	if err != nil {
+		t.Fatal(err)
+	}
+	err = env.ReaderList(nil)
+	if err == nil {
+		t.Errorf("expected error")
+	}
+}
+
 func TestEnv_ReaderCheck(t *testing.T) {
 	env := setup(t)
 	defer clean(env, t)
diff --git a/lmdb/msgfunc_test.go b/lmdb/msgfunc_test.go
new file mode 100644
index 0000000..e5539bd
--- /dev/null
+++ b/lmdb/msgfunc_test.go
@@ -0,0 +1,15 @@
+package lmdb
+
+import "testing"
+
+func TestMsgctx_conflict(t *testing.T) {
+	ctx := nextctx()
+	ctx.register(func(string) error { return nil })
+	defer func() {
+		ctx.deregister()
+		if e := recover(); e == nil {
+			t.Errorf("expeceted panic")
+		}
+	}()
+	ctx.register(func(string) error { return nil })
+}