From 3f9bcf03cc37e975a5f0f15c4a2e96a4d993ea0d Mon Sep 17 00:00:00 2001
From: Robert Vasek <robert.vasek@cern.ch>
Date: Thu, 10 Feb 2022 20:37:40 +0100
Subject: [PATCH] rados: simplified GetOmapStep and ReadOp.GetOmapValues

C strings that are passed in to rados_read_op_omap_get_vals2() are copied by
the function, see [1][2] for its implementation. It's therefore not necessary for
them to be owned by GetOmapStep. It's enough to construct and destruct them
in ReadOp.GetOmapValues.

[1] https://github.com/ceph/ceph/blob/2bd3dd512ab42f6e5d5d0dd5f5428b783681802e/src/librados/librados_c.cc#L4334-L4358
[2] https://github.com/ceph/ceph/blob/2bd3dd512ab42f6e5d5d0dd5f5428b783681802e/src/include/rados/librados.hpp#L572-L587
---
 rados/omap.go    | 24 +++---------------------
 rados/read_op.go | 14 ++++++++++----
 2 files changed, 13 insertions(+), 25 deletions(-)

diff --git a/rados/omap.go b/rados/omap.go
index 529c811..4de9c73 100644
--- a/rados/omap.go
+++ b/rados/omap.go
@@ -88,15 +88,6 @@ type OmapKeyValue struct {
 // Release method is called the public methods of the step must no longer be
 // used and may return errors.
 type GetOmapStep struct {
-	// inputs:
-	startAfter   string
-	filterPrefix string
-	maxReturn    uint64
-
-	// arguments:
-	cStartAfter   *C.char
-	cFilterPrefix *C.char
-
 	// C returned data:
 	iter C.rados_omap_iter_t
 	more *C.uchar
@@ -109,15 +100,10 @@ type GetOmapStep struct {
 	canIterate bool
 }
 
-func newGetOmapStep(startAfter, filterPrefix string, maxReturn uint64) *GetOmapStep {
+func newGetOmapStep() *GetOmapStep {
 	gos := &GetOmapStep{
-		startAfter:    startAfter,
-		filterPrefix:  filterPrefix,
-		maxReturn:     maxReturn,
-		cStartAfter:   C.CString(startAfter),
-		cFilterPrefix: C.CString(filterPrefix),
-		more:          (*C.uchar)(C.malloc(C.sizeof_uchar)),
-		rval:          (*C.int)(C.malloc(C.sizeof_int)),
+		more: (*C.uchar)(C.malloc(C.sizeof_uchar)),
+		rval: (*C.int)(C.malloc(C.sizeof_int)),
 	}
 	runtime.SetFinalizer(gos, opStepFinalizer)
 	return gos
@@ -133,10 +119,6 @@ func (gos *GetOmapStep) free() {
 	gos.more = nil
 	C.free(unsafe.Pointer(gos.rval))
 	gos.rval = nil
-	C.free(unsafe.Pointer(gos.cStartAfter))
-	gos.cStartAfter = nil
-	C.free(unsafe.Pointer(gos.cFilterPrefix))
-	gos.cFilterPrefix = nil
 }
 
 func (gos *GetOmapStep) update() error {
diff --git a/rados/read_op.go b/rados/read_op.go
index 74f2eb2..8487664 100644
--- a/rados/read_op.go
+++ b/rados/read_op.go
@@ -69,13 +69,19 @@ func (r *ReadOp) AssertExists() {
 // function. The GetOmapStep may be used to iterate over the key-value
 // pairs after the Operate call has been performed.
 func (r *ReadOp) GetOmapValues(startAfter, filterPrefix string, maxReturn uint64) *GetOmapStep {
-	gos := newGetOmapStep(startAfter, filterPrefix, maxReturn)
+	gos := newGetOmapStep()
 	r.steps = append(r.steps, gos)
+
+	cStartAfter := C.CString(startAfter)
+	cFilterPrefix := C.CString(filterPrefix)
+	defer C.free(unsafe.Pointer(cStartAfter))
+	defer C.free(unsafe.Pointer(cFilterPrefix))
+
 	C.rados_read_op_omap_get_vals2(
 		r.op,
-		gos.cStartAfter,
-		gos.cFilterPrefix,
-		C.uint64_t(gos.maxReturn),
+		cStartAfter,
+		cFilterPrefix,
+		C.uint64_t(maxReturn),
 		&gos.iter,
 		gos.more,
 		gos.rval,