rbd: eliminate invalid unsafe.Pointers

Although it seems to be stable with the current Go versions,
unsafe.Pointers that are created by applying arithmetic on nil
pointers are explicitly not allowed[1], and could potentially lead to
panics by the garbage collector.  To be on the safe side, this change
goes back to minimal inline wrappers for the callback registrations,
which should be zero-cost.  For the callbacks themselves fortunately
no wrappers are required, because CGo happily converts void* to
uintptr arguments.  The potentially problematic VoidPtr helper is
removed again.

[1] https://golang.org/pkg/unsafe/#Pointer

Signed-off-by: Sven Anderson <sven@redhat.com>

internal/cutil/cutil.go

@@ -1,14 +0,0 @@
-package cutil
-
-import "unsafe"
-
-// VoidPtr casts a uintptr value to an unsafe.Pointer value in order to use it
-// directly as a void* argument in a C function call.
-// CAUTION: NEVER store the result in a variable, or the Go GC could panic.
-func VoidPtr(i uintptr) unsafe.Pointer {
-	var nullPtr unsafe.Pointer
-	// It's not possible to cast uintptr directly to unsafe.Pointer. Therefore we
-	// cast a null pointer to uintptr and apply pointer arithmetic on it, which
-	// allows us to cast it back to unsafe.Pointer.
-	return unsafe.Pointer(uintptr(nullPtr) + i)
-}

internal/cutil/cutil_test.go

@@ -1,13 +0,0 @@
-package cutil
-
-import (
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-)
-
-func TestVoidPtr(t *testing.T) {
-	i := uintptr(42)
-	j := uintptr(VoidPtr(i))
-	assert.Equal(t, i, j)
-}

rbd/diff_iterate.go

@@ -7,8 +7,15 @@ package rbd
 #include <stdlib.h>
 #include <rbd/librbd.h>
 
-typedef int (*diff_iterate_callback_t)(uint64_t, size_t, int, void *);
-extern int diffIterateCallback(uint64_t, size_t, int, void *);
+extern int diffIterateCallback(uint64_t, size_t, int, uintptr_t);
+
+// inline wrapper to cast uintptr_t to void*
+static inline int wrap_rbd_diff_iterate2(rbd_image_t image,
+	const char *fromsnapname, uint64_t ofs, uint64_t len, uint8_t include_parent,
+	uint8_t whole_object, uintptr_t arg) {
+		return rbd_diff_iterate2(image, fromsnapname, ofs, len, include_parent,
+			whole_object, (void*)diffIterateCallback, (void*)arg);
+};
 */
 import "C"
 
@@ -16,7 +23,6 @@ import (
 	"unsafe"
 
 	"github.com/ceph/go-ceph/internal/callbacks"
-	"github.com/ceph/go-ceph/internal/cutil"
 )
 
 var diffIterateCallbacks = callbacks.New()
@@ -101,24 +107,23 @@ func (image *Image) DiffIterate(config DiffIterateConfig) error {
 	cbIndex := diffIterateCallbacks.Add(config)
 	defer diffIterateCallbacks.Remove(cbIndex)
 
-	ret := C.rbd_diff_iterate2(
+	ret := C.wrap_rbd_diff_iterate2(
 		image.image,
 		cSnapName,
 		C.uint64_t(config.Offset),
 		C.uint64_t(config.Length),
 		C.uint8_t(config.IncludeParent),
 		C.uint8_t(config.WholeObject),
-		C.diff_iterate_callback_t(C.diffIterateCallback),
-		cutil.VoidPtr(cbIndex))
+		C.uintptr_t(cbIndex))
 
 	return getError(ret)
 }
 
 //export diffIterateCallback
 func diffIterateCallback(
-	offset C.uint64_t, length C.size_t, exists C.int, index unsafe.Pointer) C.int {
+	offset C.uint64_t, length C.size_t, exists C.int, index uintptr) C.int {
 
-	v := diffIterateCallbacks.Lookup(uintptr(index))
+	v := diffIterateCallbacks.Lookup(index)
 	config := v.(DiffIterateConfig)
 	return C.int(config.Callback(
 		uint64(offset), uint64(length), int(exists), config.Data))

rbd/watchers_mimic.go

@@ -8,15 +8,19 @@ package rbd
 #cgo LDFLAGS: -lrbd
 #include <rbd/librbd.h>
 
-extern void imageWatchCallback(void *);
+extern void imageWatchCallback(uintptr_t);
+
+// inline wrapper to cast uintptr_t to void*
+static inline int wrap_rbd_update_watch(rbd_image_t image, uint64_t *handle,
+	uintptr_t arg) {
+		return rbd_update_watch(image, handle, (void*)imageWatchCallback, (void*)arg);
+	};
+
 */
 import "C"
 
 import (
-	"unsafe"
-
 	"github.com/ceph/go-ceph/internal/callbacks"
-	"github.com/ceph/go-ceph/internal/cutil"
 	"github.com/ceph/go-ceph/internal/retry"
 )
 
@@ -107,11 +111,10 @@ func (image *Image) UpdateWatch(cb WatchCallback, data interface{}) (*Watch, err
 		cbIndex: watchCallbacks.Add(wcc),
 	}
 
-	ret := C.rbd_update_watch(
+	ret := C.wrap_rbd_update_watch(
 		image.image,
 		&w.handle,
-		C.rbd_update_callback_t(C.imageWatchCallback),
-		cutil.VoidPtr(w.cbIndex))
+		C.uintptr_t(w.cbIndex))
 	if ret != 0 {
 		return nil, getError(ret)
 	}
@@ -135,8 +138,8 @@ func (w *Watch) Unwatch() error {
 }
 
 //export imageWatchCallback
-func imageWatchCallback(index unsafe.Pointer) {
-	v := watchCallbacks.Lookup(uintptr(index))
+func imageWatchCallback(index uintptr) {
+	v := watchCallbacks.Lookup(index)
 	wcc := v.(watchCallbackCtx)
 	wcc.callback(wcc.data)
 }