diff --git a/Makefile b/Makefile index ffcd9f19d..1fddd7afe 100644 --- a/Makefile +++ b/Makefile @@ -52,7 +52,14 @@ check: # Do a quick compile only check of the tests and impliclity the # library code as well. -test-binaries: cephfs.test rados.test rbd.test internal/callbacks.test internal/errutil.test internal/retry.test +test-binaries: \ + cephfs.test \ + internal/callbacks.test \ + internal/cutil.test \ + internal/errutil.test \ + internal/retry.test \ + rados.test \ + rbd.test test-bins: test-binaries %.test: % force_go_build diff --git a/cephfs/command.go b/cephfs/command.go index 0c4b3b3ef..9ed23bc96 100644 --- a/cephfs/command.go +++ b/cephfs/command.go @@ -10,8 +10,14 @@ import "C" import ( "unsafe" + + "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) @@ -36,51 +42,22 @@ func (mount *MountInfo) MdsCommandWithInputBuffer(mdsSpec string, args [][]byte, func (mount *MountInfo) mdsCommand(mdsSpec string, args [][]byte, inputBuffer []byte) (buffer []byte, info string, err error) { spec := C.CString(mdsSpec) defer C.free(unsafe.Pointer(spec)) - - argc := len(args) - argv := make([]*C.char, argc) - - for i, arg := range args { - argv[i] = C.CString(string(arg)) - } - // free all array elements in a single defer - defer func() { - for i := range argv { - C.free(unsafe.Pointer(argv[i])) - } - }() - - var ( - outs, outbuf *C.char - outslen, outbuflen C.size_t - ) - inbuf := C.CString(string(inputBuffer)) - inbufLen := len(inputBuffer) - defer C.free(unsafe.Pointer(inbuf)) + ci := cutil.NewCommandInput(args, inputBuffer) + defer ci.Free() + co := cutil.NewCommandOutput().SetFreeFunc(cephBufferFree) + defer co.Free() ret := C.ceph_mds_command( - mount.mount, // cephfs mount ref - spec, // mds spec - &argv[0], // cmd array - C.size_t(argc), // cmd array length - inbuf, // bulk input - C.size_t(inbufLen), // length inbuf - &outbuf, // buffer - &outbuflen, // buffer length - &outs, // status string - &outslen) - - if outslen > 0 { - info = C.GoStringN(outs, C.int(outslen)) - C.free(unsafe.Pointer(outs)) - } - if outbuflen > 0 { - buffer = C.GoBytes(unsafe.Pointer(outbuf), C.int(outbuflen)) - C.free(unsafe.Pointer(outbuf)) - } - if ret != 0 { - return nil, info, getError(ret) - } - - return buffer, info, nil + mount.mount, // cephfs mount ref + spec, // mds spec + (**C.char)(ci.Cmd()), + C.size_t(ci.CmdLen()), + (*C.char)(ci.InBuf()), + C.size_t(ci.InBufLen()), + (**C.char)(co.OutBuf()), + (*C.size_t)(co.OutBufLen()), + (**C.char)(co.Outs()), + (*C.size_t)(co.OutsLen())) + buf, status := co.GoValues() + return buf, status, getError(ret) } diff --git a/entrypoint.sh b/entrypoint.sh index 7e920453e..78f653eec 100755 --- a/entrypoint.sh +++ b/entrypoint.sh @@ -175,6 +175,7 @@ test_go_ceph() { pkgs=(\ "cephfs" \ "internal/callbacks" \ + "internal/cutil" \ "internal/errutil" \ "internal/retry" \ "rados" \ diff --git a/internal/cutil/command_input.go b/internal/cutil/command_input.go new file mode 100644 index 000000000..fc11b82ea --- /dev/null +++ b/internal/cutil/command_input.go @@ -0,0 +1,62 @@ +package cutil + +/* +#include +*/ +import "C" + +import ( + "unsafe" +) + +// CommandInput can be used to manage the input to ceph's *_command functions. +type CommandInput struct { + cmd []*C.char + inbuf []byte +} + +// NewCommandInput creates C-level pointers from go byte buffers suitable +// for passing off to ceph's *_command functions. +func NewCommandInput(cmd [][]byte, inputBuffer []byte) *CommandInput { + ci := &CommandInput{ + cmd: make([]*C.char, len(cmd)), + inbuf: inputBuffer, + } + for i := range cmd { + ci.cmd[i] = C.CString(string(cmd[i])) + } + return ci +} + +// Free any C memory managed by this CommandInput. +func (ci *CommandInput) Free() { + for i := range ci.cmd { + C.free(unsafe.Pointer(ci.cmd[i])) + } + ci.cmd = nil +} + +// Cmd returns an unsafe wrapper around an array of C-strings. +func (ci *CommandInput) Cmd() CharPtrPtr { + ptr := &ci.cmd[0] + return CharPtrPtr(ptr) +} + +// CmdLen returns the length of the array of C-strings returned by +// Cmd. +func (ci *CommandInput) CmdLen() SizeT { + return SizeT(len(ci.cmd)) +} + +// InBuf returns an unsafe wrapper to a C char*. +func (ci *CommandInput) InBuf() CharPtr { + if len(ci.inbuf) == 0 { + return nil + } + return CharPtr(&ci.inbuf[0]) +} + +// InBufLen returns the length of the buffer returned by InBuf. +func (ci *CommandInput) InBufLen() SizeT { + return SizeT(len(ci.inbuf)) +} diff --git a/internal/cutil/command_input_test.go b/internal/cutil/command_input_test.go new file mode 100644 index 000000000..60057a014 --- /dev/null +++ b/internal/cutil/command_input_test.go @@ -0,0 +1,50 @@ +package cutil + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestCommandInput(t *testing.T) { + t.Run("newAndFree", func(t *testing.T) { + ci := NewCommandInput( + [][]byte{[]byte("foobar")}, + nil) + ci.Free() + }) + t.Run("cmd", func(t *testing.T) { + ci := NewCommandInput( + [][]byte{[]byte("foobar")}, + nil) + defer ci.Free() + assert.Len(t, ci.cmd, 1) + assert.EqualValues(t, 1, ci.CmdLen()) + assert.NotNil(t, ci.Cmd()) + }) + t.Run("cmd2", func(t *testing.T) { + ci := NewCommandInput( + [][]byte{[]byte("foobar"), []byte("snarf")}, + nil) + defer ci.Free() + assert.Len(t, ci.cmd, 2) + assert.EqualValues(t, 2, ci.CmdLen()) + assert.NotNil(t, ci.Cmd()) + }) + t.Run("noInBuf", func(t *testing.T) { + ci := NewCommandInput( + [][]byte{[]byte("foobar")}, + nil) + defer ci.Free() + assert.EqualValues(t, 0, ci.InBufLen()) + assert.Equal(t, CharPtr(nil), ci.InBuf()) + }) + t.Run("hasInBuf", func(t *testing.T) { + ci := NewCommandInput( + [][]byte{[]byte("foobar")}, + []byte("original oregano")) + defer ci.Free() + assert.EqualValues(t, 16, ci.InBufLen()) + assert.NotEqual(t, CharPtr(nil), ci.InBuf()) + }) +} diff --git a/internal/cutil/command_output.go b/internal/cutil/command_output.go new file mode 100644 index 000000000..8b0c6e529 --- /dev/null +++ b/internal/cutil/command_output.go @@ -0,0 +1,100 @@ +package cutil + +/* +#include +*/ +import "C" + +import ( + "unsafe" +) + +// 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 + outsLen C.size_t +} + +// 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{ + free: free, + } +} + +// SetFreeFunc sets the function used to free memory held by CommandOutput. +// Not all uses of CommandOutput expect to use the basic C.free function +// and either require or prefer the use of a custom deallocation function. +// Use SetFreeFunc to change the free function and return the modified +// CommandOutput object. +func (co *CommandOutput) SetFreeFunc(f FreeFunc) *CommandOutput { + co.free = f + return co +} + +// Free any C memory tracked by this object. +func (co *CommandOutput) Free() { + if co.outBuf != nil { + co.free(unsafe.Pointer(co.outBuf)) + } + if co.outs != nil { + co.free(unsafe.Pointer(co.outs)) + } +} + +// OutBuf returns an unsafe wrapper around a pointer to a `char*`. +func (co *CommandOutput) OutBuf() CharPtrPtr { + return CharPtrPtr(&co.outBuf) +} + +// OutBufLen returns an unsafe wrapper around a pointer to a size_t. +func (co *CommandOutput) OutBufLen() SizeTPtr { + return SizeTPtr(&co.outBufLen) +} + +// Outs returns an unsafe wrapper around a pointer to a `char*`. +func (co *CommandOutput) Outs() CharPtrPtr { + return CharPtrPtr(&co.outs) +} + +// OutsLen returns an unsafe wrapper around a pointer to a size_t. +func (co *CommandOutput) OutsLen() SizeTPtr { + return SizeTPtr(&co.outsLen) +} + +// GoValues returns native go values converted from the internal C-language +// values tracked by this object. +func (co *CommandOutput) GoValues() (buf []byte, status string) { + if co.outBufLen > 0 { + buf = C.GoBytes(unsafe.Pointer(co.outBuf), C.int(co.outBufLen)) + } + if co.outsLen > 0 { + status = C.GoStringN(co.outs, C.int(co.outsLen)) + } + return buf, status +} + +// testSetString is only used by the unit tests for this file. +// It is located here due to the restriction on having import "C" in +// go test files. :-( +// It mimics a C function that takes a pointer to a +// string and length and allocates memory and sets the pointers +// to the new string and its length. +func testSetString(strp CharPtrPtr, lenp SizeTPtr, s string) { + sp := (**C.char)(strp) + lp := (*C.size_t)(lenp) + *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) +} diff --git a/internal/cutil/command_output_test.go b/internal/cutil/command_output_test.go new file mode 100644 index 000000000..74278b1bb --- /dev/null +++ b/internal/cutil/command_output_test.go @@ -0,0 +1,56 @@ +package cutil + +import ( + "testing" + "unsafe" + + "github.com/stretchr/testify/assert" +) + +func TestCommandOutput(t *testing.T) { + t.Run("newAndFree", func(t *testing.T) { + co := NewCommandOutput() + assert.NotNil(t, co) + co.Free() + }) + t.Run("setValues", func(t *testing.T) { + co := NewCommandOutput() + assert.NotNil(t, co) + defer co.Free() + testSetString(co.OutBuf(), co.OutBufLen(), "i got style") + testSetString(co.Outs(), co.OutsLen(), "i got rhythm") + b, s := co.GoValues() + assert.EqualValues(t, []byte("i got style"), b) + assert.EqualValues(t, "i got rhythm", s) + }) + t.Run("setOnlyOutBuf", func(t *testing.T) { + co := NewCommandOutput() + assert.NotNil(t, co) + defer co.Free() + testSetString(co.OutBuf(), co.OutBufLen(), "i got style") + b, s := co.GoValues() + assert.EqualValues(t, []byte("i got style"), b) + assert.EqualValues(t, "", s) + }) + t.Run("setOnlyOuts", func(t *testing.T) { + co := NewCommandOutput() + assert.NotNil(t, co) + defer co.Free() + testSetString(co.Outs(), co.OutsLen(), "i got rhythm") + b, s := co.GoValues() + assert.Nil(t, b) + assert.EqualValues(t, "i got rhythm", s) + }) + t.Run("customFreeFunc", func(t *testing.T) { + callCount := 0 + co := NewCommandOutput().SetFreeFunc(func(p unsafe.Pointer) { + callCount++ + free(p) + }) + assert.NotNil(t, co) + testSetString(co.OutBuf(), co.OutBufLen(), "i got style") + testSetString(co.Outs(), co.OutsLen(), "i got rhythm") + co.Free() + assert.Equal(t, 2, callCount) + }) +} diff --git a/internal/cutil/type_aliases.go b/internal/cutil/type_aliases.go new file mode 100644 index 000000000..79495ef98 --- /dev/null +++ b/internal/cutil/type_aliases.go @@ -0,0 +1,28 @@ +package cutil + +import "C" + +import ( + "unsafe" +) + +// Basic types from C that we can make "public" without too much fuss. + +// SizeT wraps size_t from C. +type SizeT C.size_t + +// This section contains a bunch of types that are basically just +// unsafe.Pointer but have specific types to help "self document" what the +// underlying pointer is really meant to represent. + +// CharPtrPtr is an unsafe pointer wrapping C's `char**`. +type CharPtrPtr unsafe.Pointer + +// CharPtr is an unsafe pointer wrapping C's `char*`. +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) diff --git a/rados/command.go b/rados/command.go index ec41d1cf6..1ec0422cd 100644 --- a/rados/command.go +++ b/rados/command.go @@ -7,8 +7,14 @@ import "C" import ( "unsafe" + + "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) @@ -20,42 +26,23 @@ func (c *Conn) MonCommandWithInputBuffer(args, inputBuffer []byte) ([]byte, stri } func (c *Conn) monCommand(args, inputBuffer []byte) ([]byte, string, error) { - argv := C.CString(string(args)) - defer C.free(unsafe.Pointer(argv)) - - var ( - info string - buffer []byte - outs, outbuf *C.char - outslen, outbuflen C.size_t - ) - inbuf := C.CString(string(inputBuffer)) - inbufLen := len(inputBuffer) - defer C.free(unsafe.Pointer(inbuf)) - - ret := C.rados_mon_command(c.cluster, - &argv, 1, - inbuf, // bulk input (e.g. crush map) - C.size_t(inbufLen), // length inbuf - &outbuf, // buffer - &outbuflen, // buffer length - &outs, // status string - &outslen) - - if outslen > 0 { - info = C.GoStringN(outs, C.int(outslen)) - C.free(unsafe.Pointer(outs)) - } - if outbuflen > 0 { - buffer = C.GoBytes(unsafe.Pointer(outbuf), C.int(outbuflen)) - C.free(unsafe.Pointer(outbuf)) - } - if ret != 0 { - err := getError(ret) - return nil, info, err - } - - return buffer, info, nil + ci := cutil.NewCommandInput([][]byte{args}, inputBuffer) + defer ci.Free() + co := cutil.NewCommandOutput().SetFreeFunc(radosBufferFree) + defer co.Free() + + ret := C.rados_mon_command( + c.cluster, + (**C.char)(ci.Cmd()), + C.size_t(ci.CmdLen()), + (*C.char)(ci.InBuf()), + C.size_t(ci.InBufLen()), + (**C.char)(co.OutBuf()), + (*C.size_t)(co.OutBufLen()), + (**C.char)(co.Outs()), + (*C.size_t)(co.OutsLen())) + buf, status := co.GoValues() + return buf, status, getError(ret) } // PGCommand sends a command to one of the PGs @@ -85,50 +72,24 @@ func (c *Conn) PGCommandWithInputBuffer(pgid []byte, args [][]byte, inputBuffer func (c *Conn) pgCommand(pgid []byte, args [][]byte, inputBuffer []byte) ([]byte, string, error) { name := C.CString(string(pgid)) defer C.free(unsafe.Pointer(name)) + ci := cutil.NewCommandInput(args, inputBuffer) + defer ci.Free() + co := cutil.NewCommandOutput().SetFreeFunc(radosBufferFree) + defer co.Free() - argc := len(args) - argv := make([]*C.char, argc) - - for i, arg := range args { - argv[i] = C.CString(string(arg)) - defer C.free(unsafe.Pointer(argv[i])) - } - - var ( - info string - buffer []byte - outs, outbuf *C.char - outslen, outbuflen C.size_t - ) - inbuf := C.CString(string(inputBuffer)) - inbufLen := len(inputBuffer) - defer C.free(unsafe.Pointer(inbuf)) - - ret := C.rados_pg_command(c.cluster, + ret := C.rados_pg_command( + c.cluster, name, - &argv[0], - C.size_t(argc), - inbuf, // bulk input - C.size_t(inbufLen), // length inbuf - &outbuf, // buffer - &outbuflen, // buffer length - &outs, // status string - &outslen) - - if outslen > 0 { - info = C.GoStringN(outs, C.int(outslen)) - C.free(unsafe.Pointer(outs)) - } - if outbuflen > 0 { - buffer = C.GoBytes(unsafe.Pointer(outbuf), C.int(outbuflen)) - C.free(unsafe.Pointer(outbuf)) - } - if ret != 0 { - err := getError(ret) - return nil, info, err - } - - return buffer, info, nil + (**C.char)(ci.Cmd()), + C.size_t(ci.CmdLen()), + (*C.char)(ci.InBuf()), + C.size_t(ci.InBufLen()), + (**C.char)(co.OutBuf()), + (*C.size_t)(co.OutBufLen()), + (**C.char)(co.Outs()), + (*C.size_t)(co.OutsLen())) + buf, status := co.GoValues() + return buf, status, getError(ret) } // MgrCommand sends a command to a ceph-mgr. @@ -148,47 +109,21 @@ func (c *Conn) MgrCommandWithInputBuffer(args [][]byte, inputBuffer []byte) ([]b // size_t *outbuflen, char **outs, // size_t *outslen); func (c *Conn) mgrCommand(args [][]byte, inputBuffer []byte) ([]byte, string, error) { - argc := len(args) - argv := make([]*C.char, argc) - - for i, arg := range args { - argv[i] = C.CString(string(arg)) - defer C.free(unsafe.Pointer(argv[i])) - } - - var ( - info string - buffer []byte - outs, outbuf *C.char - outslen, outbuflen C.size_t - ) - inbuf := C.CString(string(inputBuffer)) - inbufLen := len(inputBuffer) - defer C.free(unsafe.Pointer(inbuf)) + ci := cutil.NewCommandInput(args, inputBuffer) + defer ci.Free() + co := cutil.NewCommandOutput().SetFreeFunc(radosBufferFree) + defer co.Free() ret := C.rados_mgr_command( c.cluster, - &argv[0], - C.size_t(argc), - inbuf, - C.size_t(inbufLen), - &outbuf, - &outbuflen, - &outs, - &outslen) - - if outslen > 0 { - info = C.GoStringN(outs, C.int(outslen)) - C.free(unsafe.Pointer(outs)) - } - if outbuflen > 0 { - buffer = C.GoBytes(unsafe.Pointer(outbuf), C.int(outbuflen)) - C.free(unsafe.Pointer(outbuf)) - } - if ret != 0 { - err := getError(ret) - return nil, info, err - } - - return buffer, info, nil + (**C.char)(ci.Cmd()), + C.size_t(ci.CmdLen()), + (*C.char)(ci.InBuf()), + C.size_t(ci.InBufLen()), + (**C.char)(co.OutBuf()), + (*C.size_t)(co.OutBufLen()), + (**C.char)(co.Outs()), + (*C.size_t)(co.OutsLen())) + buf, status := co.GoValues() + return buf, status, getError(ret) }