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 <[email protected]>
  • Loading branch information
phlogistonjohn committed Apr 28, 2020
1 parent 0510a73 commit 87589cc
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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 87589cc

Please sign in to comment.