Skip to content

Commit

Permalink
cutil: allow passing free functions to command output type
Browse files Browse the repository at this point in the history
The *_command functions in librados and libcephfs document the use
of specific free functions for data allocated. These functions are
currently just wrappers around C's free() function. However, to be
more strictly compliant this change adds a free-function callback
to the CommandOutput type and the specific free functions are now
used outside the unit tests.

Signed-off-by: John Mulligan <jmulligan@redhat.com>
  • Loading branch information
phlogistonjohn committed Apr 23, 2020
1 parent c8ac611 commit 5c086df
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 12 deletions.
6 changes: 5 additions & 1 deletion cephfs/command.go
Expand Up @@ -14,6 +14,10 @@ import (
"github.com/ceph/go-ceph/internal/cutil"
)

func cephBufferFree(p unsafe.Pointer) {
C.ceph_buffer_free((*C.char)(p))
}

// MdsCommand sends commands to the specified MDS.
func (mount *MountInfo) MdsCommand(mdsSpec string, args [][]byte) ([]byte, string, error) {
return mount.mdsCommand(mdsSpec, args, nil)
Expand All @@ -40,7 +44,7 @@ func (mount *MountInfo) mdsCommand(mdsSpec string, args [][]byte, inputBuffer []
defer C.free(unsafe.Pointer(spec))
ci := cutil.NewCommandInput(args, inputBuffer)
defer ci.Free()
co := cutil.NewCommandOutput()
co := cutil.NewCommandOutput(cephBufferFree)
defer co.Free()

ret := C.ceph_mds_command(
Expand Down
17 changes: 13 additions & 4 deletions internal/cutil/command_output.go
Expand Up @@ -12,6 +12,7 @@ import (
// CommandOutput can be used to manage the outputs of ceph's *_command
// functions.
type CommandOutput struct {
free FreeFunc
outBuf *C.char
outBufLen C.size_t
outs *C.char
Expand All @@ -21,17 +22,19 @@ type CommandOutput struct {
// NewCommandOutput returns an empty CommandOutput. The pointers that
// a CommandOutput provides can be used to get the results of ceph's
// *_command functions.
func NewCommandOutput() *CommandOutput {
return &CommandOutput{}
func NewCommandOutput(free FreeFunc) *CommandOutput {
return &CommandOutput{
free: free,
}
}

// Free any C memory tracked by this object.
func (co *CommandOutput) Free() {
if co.outBuf != nil {
C.free(unsafe.Pointer(co.outBuf))
co.free(unsafe.Pointer(co.outBuf))
}
if co.outs != nil {
C.free(unsafe.Pointer(co.outs))
co.free(unsafe.Pointer(co.outs))
}
}

Expand Down Expand Up @@ -79,3 +82,9 @@ func testSetString(strp CharPtrPtr, lenp SizeTPtr, s string) {
*sp = C.CString(s)
*lp = C.size_t(len(s))
}

// free wraps C.free.
// Required for unit tests that may not use cgo directly.
func free(p unsafe.Pointer) {
C.free(p)
}
8 changes: 4 additions & 4 deletions internal/cutil/command_output_test.go
Expand Up @@ -8,12 +8,12 @@ import (

func TestCommandOutput(t *testing.T) {
t.Run("newAndFree", func(t *testing.T) {
co := NewCommandOutput()
co := NewCommandOutput(free)
assert.NotNil(t, co)
co.Free()
})
t.Run("setValues", func(t *testing.T) {
co := NewCommandOutput()
co := NewCommandOutput(free)
assert.NotNil(t, co)
defer co.Free()
testSetString(co.OutBuf(), co.OutBufLen(), "i got style")
Expand All @@ -23,7 +23,7 @@ func TestCommandOutput(t *testing.T) {
assert.EqualValues(t, "i got rhythm", s)
})
t.Run("setOnlyOutBuf", func(t *testing.T) {
co := NewCommandOutput()
co := NewCommandOutput(free)
assert.NotNil(t, co)
defer co.Free()
testSetString(co.OutBuf(), co.OutBufLen(), "i got style")
Expand All @@ -32,7 +32,7 @@ func TestCommandOutput(t *testing.T) {
assert.EqualValues(t, "", s)
})
t.Run("setOnlyOuts", func(t *testing.T) {
co := NewCommandOutput()
co := NewCommandOutput(free)
assert.NotNil(t, co)
defer co.Free()
testSetString(co.Outs(), co.OutsLen(), "i got rhythm")
Expand Down
3 changes: 3 additions & 0 deletions internal/cutil/type_aliases.go
Expand Up @@ -23,3 +23,6 @@ type CharPtr unsafe.Pointer

// SizeTPtr is an unsafe pointer wrapping C's `size_t*`.
type SizeTPtr unsafe.Pointer

// FreeFunc is a wrapper around calls to, or act like, C's free function.
type FreeFunc func(unsafe.Pointer)
10 changes: 7 additions & 3 deletions rados/command.go
Expand Up @@ -11,6 +11,10 @@ import (
"github.com/ceph/go-ceph/internal/cutil"
)

func radosBufferFree(p unsafe.Pointer) {
C.rados_buffer_free((*C.char)(p))
}

// MonCommand sends a command to one of the monitors
func (c *Conn) MonCommand(args []byte) ([]byte, string, error) {
return c.monCommand(args, nil)
Expand All @@ -24,7 +28,7 @@ func (c *Conn) MonCommandWithInputBuffer(args, inputBuffer []byte) ([]byte, stri
func (c *Conn) monCommand(args, inputBuffer []byte) ([]byte, string, error) {
ci := cutil.NewCommandInput([][]byte{args}, inputBuffer)
defer ci.Free()
co := cutil.NewCommandOutput()
co := cutil.NewCommandOutput(radosBufferFree)
defer co.Free()

ret := C.rados_mon_command(
Expand Down Expand Up @@ -70,7 +74,7 @@ func (c *Conn) pgCommand(pgid []byte, args [][]byte, inputBuffer []byte) ([]byte
defer C.free(unsafe.Pointer(name))
ci := cutil.NewCommandInput(args, inputBuffer)
defer ci.Free()
co := cutil.NewCommandOutput()
co := cutil.NewCommandOutput(radosBufferFree)
defer co.Free()

ret := C.rados_pg_command(
Expand Down Expand Up @@ -107,7 +111,7 @@ func (c *Conn) MgrCommandWithInputBuffer(args [][]byte, inputBuffer []byte) ([]b
func (c *Conn) mgrCommand(args [][]byte, inputBuffer []byte) ([]byte, string, error) {
ci := cutil.NewCommandInput(args, inputBuffer)
defer ci.Free()
co := cutil.NewCommandOutput()
co := cutil.NewCommandOutput(radosBufferFree)
defer co.Free()

ret := C.rados_mgr_command(
Expand Down

0 comments on commit 5c086df

Please sign in to comment.