From 0e8de3d155c933ebff9aa650e3abe6c39c480d21 Mon Sep 17 00:00:00 2001 From: leo Date: Mon, 18 Nov 2024 21:03:47 +0800 Subject: [PATCH] fix: empty property in msg (#291) Co-authored-by: Hu Yueh-Wei --- .../binding/go/interface/ten/msg_property.go | 10 + .../go/interface/ten/ten_env_property.go | 426 +++++++++--------- .../binding/go/interface/ten/value.go | 102 ----- .../extension/extension_a/extension.go | 23 + .../extension/extension_b/extension.go | 9 + 5 files changed, 258 insertions(+), 312 deletions(-) diff --git a/core/src/ten_runtime/binding/go/interface/ten/msg_property.go b/core/src/ten_runtime/binding/go/interface/ten/msg_property.go index a387c1217..adbd701b8 100644 --- a/core/src/ten_runtime/binding/go/interface/ten/msg_property.go +++ b/core/src/ten_runtime/binding/go/interface/ten/msg_property.go @@ -65,6 +65,11 @@ func (p *msg) getPropertyString(path string) (string, error) { ) } + if pSize == 0 { + // We can not allocate a []byte with size 0, so just return "". + return "", nil + } + return getPropStr( uintptr(pSize), func(v unsafe.Pointer) C.ten_go_status_t { @@ -116,6 +121,11 @@ func (p *msg) getPropertyBytes(path string) ([]byte, error) { ) } + if pSize == 0 { + // We can not allocate a []byte with size 0, so just return nil. + return nil, nil + } + return getPropBytes( uintptr(pSize), func(v unsafe.Pointer) C.ten_go_status_t { diff --git a/core/src/ten_runtime/binding/go/interface/ten/ten_env_property.go b/core/src/ten_runtime/binding/go/interface/ten/ten_env_property.go index de4be9a09..fcdf7640d 100644 --- a/core/src/ten_runtime/binding/go/interface/ten/ten_env_property.go +++ b/core/src/ten_runtime/binding/go/interface/ten/ten_env_property.go @@ -399,214 +399,216 @@ func (p *tenEnv) SetProperty(path string, value any) error { return err } - // Create a channel to wait for the async operation in C to complete. - done := make(chan error, 1) - callbackHandle := newGoHandle(done) - - var err error - switch pt { - case propTypeBool: - apiStatus := C.ten_go_ten_env_set_property_bool( - p.cPtr, - unsafe.Pointer(unsafe.StringData(path)), - C.int(len(path)), - C.bool(value.(bool)), - C.uintptr_t(callbackHandle), - ) - err = withGoStatus(&apiStatus) - - case propTypeInt8: - apiStatus := C.ten_go_ten_env_set_property_int8( - p.cPtr, - unsafe.Pointer(unsafe.StringData(path)), - C.int(len(path)), - C.int8_t(value.(int8)), - C.uintptr_t(callbackHandle), - ) - err = withGoStatus(&apiStatus) - - case propTypeInt16: - apiStatus := C.ten_go_ten_env_set_property_int16( - p.cPtr, - unsafe.Pointer(unsafe.StringData(path)), - C.int(len(path)), - C.int16_t(value.(int16)), - C.uintptr_t(callbackHandle), - ) - err = withGoStatus(&apiStatus) - - case propTypeInt32: - apiStatus := C.ten_go_ten_env_set_property_int32( - p.cPtr, - unsafe.Pointer(unsafe.StringData(path)), - C.int(len(path)), - C.int32_t(value.(int32)), - C.uintptr_t(callbackHandle), - ) - err = withGoStatus(&apiStatus) + return withCGO(func() error { + // Create a channel to wait for the async operation in C to complete. + done := make(chan error, 1) + callbackHandle := newGoHandle(done) + + var err error + switch pt { + case propTypeBool: + apiStatus := C.ten_go_ten_env_set_property_bool( + p.cPtr, + unsafe.Pointer(unsafe.StringData(path)), + C.int(len(path)), + C.bool(value.(bool)), + C.uintptr_t(callbackHandle), + ) + err = withGoStatus(&apiStatus) - case propTypeInt64: - apiStatus := C.ten_go_ten_env_set_property_int64( - p.cPtr, - unsafe.Pointer(unsafe.StringData(path)), - C.int(len(path)), - C.int64_t(value.(int64)), - C.uintptr_t(callbackHandle), - ) - err = withGoStatus(&apiStatus) + case propTypeInt8: + apiStatus := C.ten_go_ten_env_set_property_int8( + p.cPtr, + unsafe.Pointer(unsafe.StringData(path)), + C.int(len(path)), + C.int8_t(value.(int8)), + C.uintptr_t(callbackHandle), + ) + err = withGoStatus(&apiStatus) - case propTypeInt: - if is64bit { - apiStatus := C.ten_go_ten_env_set_property_int64( + case propTypeInt16: + apiStatus := C.ten_go_ten_env_set_property_int16( p.cPtr, unsafe.Pointer(unsafe.StringData(path)), C.int(len(path)), - C.int64_t(value.(int)), + C.int16_t(value.(int16)), C.uintptr_t(callbackHandle), ) err = withGoStatus(&apiStatus) - } else { + + case propTypeInt32: apiStatus := C.ten_go_ten_env_set_property_int32( p.cPtr, unsafe.Pointer(unsafe.StringData(path)), C.int(len(path)), - C.int32_t(value.(int)), + C.int32_t(value.(int32)), C.uintptr_t(callbackHandle), ) err = withGoStatus(&apiStatus) - } - case propTypeUint8: - apiStatus := C.ten_go_ten_env_set_property_uint8( - p.cPtr, - unsafe.Pointer(unsafe.StringData(path)), - C.int(len(path)), - C.uint8_t(value.(uint8)), - C.uintptr_t(callbackHandle), - ) - err = withGoStatus(&apiStatus) + case propTypeInt64: + apiStatus := C.ten_go_ten_env_set_property_int64( + p.cPtr, + unsafe.Pointer(unsafe.StringData(path)), + C.int(len(path)), + C.int64_t(value.(int64)), + C.uintptr_t(callbackHandle), + ) + err = withGoStatus(&apiStatus) - case propTypeUint16: - apiStatus := C.ten_go_ten_env_set_property_uint16( - p.cPtr, - unsafe.Pointer(unsafe.StringData(path)), - C.int(len(path)), - C.uint16_t(value.(uint16)), - C.uintptr_t(callbackHandle), - ) - err = withGoStatus(&apiStatus) + case propTypeInt: + if is64bit { + apiStatus := C.ten_go_ten_env_set_property_int64( + p.cPtr, + unsafe.Pointer(unsafe.StringData(path)), + C.int(len(path)), + C.int64_t(value.(int)), + C.uintptr_t(callbackHandle), + ) + err = withGoStatus(&apiStatus) + } else { + apiStatus := C.ten_go_ten_env_set_property_int32( + p.cPtr, + unsafe.Pointer(unsafe.StringData(path)), + C.int(len(path)), + C.int32_t(value.(int)), + C.uintptr_t(callbackHandle), + ) + err = withGoStatus(&apiStatus) + } + + case propTypeUint8: + apiStatus := C.ten_go_ten_env_set_property_uint8( + p.cPtr, + unsafe.Pointer(unsafe.StringData(path)), + C.int(len(path)), + C.uint8_t(value.(uint8)), + C.uintptr_t(callbackHandle), + ) + err = withGoStatus(&apiStatus) - case propTypeUint32: - apiStatus := C.ten_go_ten_env_set_property_uint32( - p.cPtr, - unsafe.Pointer(unsafe.StringData(path)), - C.int(len(path)), - C.uint32_t(value.(uint32)), - C.uintptr_t(callbackHandle), - ) - err = withGoStatus(&apiStatus) + case propTypeUint16: + apiStatus := C.ten_go_ten_env_set_property_uint16( + p.cPtr, + unsafe.Pointer(unsafe.StringData(path)), + C.int(len(path)), + C.uint16_t(value.(uint16)), + C.uintptr_t(callbackHandle), + ) + err = withGoStatus(&apiStatus) - case propTypeUint64: - apiStatus := C.ten_go_ten_env_set_property_uint64( - p.cPtr, - unsafe.Pointer(unsafe.StringData(path)), - C.int(len(path)), - C.uint64_t(value.(uint64)), - C.uintptr_t(callbackHandle), - ) - err = withGoStatus(&apiStatus) + case propTypeUint32: + apiStatus := C.ten_go_ten_env_set_property_uint32( + p.cPtr, + unsafe.Pointer(unsafe.StringData(path)), + C.int(len(path)), + C.uint32_t(value.(uint32)), + C.uintptr_t(callbackHandle), + ) + err = withGoStatus(&apiStatus) - case propTypeUint: - if is64bit { + case propTypeUint64: apiStatus := C.ten_go_ten_env_set_property_uint64( p.cPtr, unsafe.Pointer(unsafe.StringData(path)), C.int(len(path)), - C.uint64_t(value.(uint)), + C.uint64_t(value.(uint64)), C.uintptr_t(callbackHandle), ) err = withGoStatus(&apiStatus) - } else { - apiStatus := C.ten_go_ten_env_set_property_uint32( + + case propTypeUint: + if is64bit { + apiStatus := C.ten_go_ten_env_set_property_uint64( + p.cPtr, + unsafe.Pointer(unsafe.StringData(path)), + C.int(len(path)), + C.uint64_t(value.(uint)), + C.uintptr_t(callbackHandle), + ) + err = withGoStatus(&apiStatus) + } else { + apiStatus := C.ten_go_ten_env_set_property_uint32( + p.cPtr, + unsafe.Pointer(unsafe.StringData(path)), + C.int(len(path)), + C.uint32_t(value.(uint)), + C.uintptr_t(callbackHandle), + ) + err = withGoStatus(&apiStatus) + } + + case propTypeFloat32: + apiStatus := C.ten_go_ten_env_set_property_float32( p.cPtr, unsafe.Pointer(unsafe.StringData(path)), C.int(len(path)), - C.uint32_t(value.(uint)), + C.float(value.(float32)), C.uintptr_t(callbackHandle), ) err = withGoStatus(&apiStatus) - } - case propTypeFloat32: - apiStatus := C.ten_go_ten_env_set_property_float32( - p.cPtr, - unsafe.Pointer(unsafe.StringData(path)), - C.int(len(path)), - C.float(value.(float32)), - C.uintptr_t(callbackHandle), - ) - err = withGoStatus(&apiStatus) - - case propTypeFloat64: - apiStatus := C.ten_go_ten_env_set_property_float64( - p.cPtr, - unsafe.Pointer(unsafe.StringData(path)), - C.int(len(path)), - C.double(value.(float64)), - C.uintptr_t(callbackHandle), - ) - err = withGoStatus(&apiStatus) + case propTypeFloat64: + apiStatus := C.ten_go_ten_env_set_property_float64( + p.cPtr, + unsafe.Pointer(unsafe.StringData(path)), + C.int(len(path)), + C.double(value.(float64)), + C.uintptr_t(callbackHandle), + ) + err = withGoStatus(&apiStatus) - case propTypeStr: - vs := value.(string) - apiStatus := C.ten_go_ten_env_set_property_string( - p.cPtr, - unsafe.Pointer(unsafe.StringData(path)), - C.int(len(path)), - unsafe.Pointer(unsafe.StringData(vs)), - C.int(len(vs)), - C.uintptr_t(callbackHandle), - ) - err = withGoStatus(&apiStatus) + case propTypeStr: + vs := value.(string) + apiStatus := C.ten_go_ten_env_set_property_string( + p.cPtr, + unsafe.Pointer(unsafe.StringData(path)), + C.int(len(path)), + unsafe.Pointer(unsafe.StringData(vs)), + C.int(len(vs)), + C.uintptr_t(callbackHandle), + ) + err = withGoStatus(&apiStatus) - case propTypeBuf: - vb := value.([]byte) - apiStatus := C.ten_go_ten_env_set_property_buf( - p.cPtr, - unsafe.Pointer(unsafe.StringData(path)), - C.int(len(path)), - unsafe.Pointer(unsafe.SliceData(vb)), - C.int(len(vb)), - C.uintptr_t(callbackHandle), - ) - err = withGoStatus(&apiStatus) + case propTypeBuf: + vb := value.([]byte) + apiStatus := C.ten_go_ten_env_set_property_buf( + p.cPtr, + unsafe.Pointer(unsafe.StringData(path)), + C.int(len(path)), + unsafe.Pointer(unsafe.SliceData(vb)), + C.int(len(vb)), + C.uintptr_t(callbackHandle), + ) + err = withGoStatus(&apiStatus) - case propTypePtr: - vh := newGoHandle(value) - apiStatus := C.ten_go_ten_env_set_property_ptr( - p.cPtr, - unsafe.Pointer(unsafe.StringData(path)), - C.int(len(path)), - cHandle(vh), - C.uintptr_t(callbackHandle), - ) - err = withGoStatus(&apiStatus) + case propTypePtr: + vh := newGoHandle(value) + apiStatus := C.ten_go_ten_env_set_property_ptr( + p.cPtr, + unsafe.Pointer(unsafe.StringData(path)), + C.int(len(path)), + cHandle(vh), + C.uintptr_t(callbackHandle), + ) + err = withGoStatus(&apiStatus) - default: - // Should not happen. - err = newTenError(ErrnoInvalidType, "") - } + default: + // Should not happen. + err = newTenError(ErrnoInvalidType, "") + } - if err != nil { - // Clean up the handle if there was an error. - loadAndDeleteGoHandle(callbackHandle) - return err - } + if err != nil { + // Clean up the handle if there was an error. + loadAndDeleteGoHandle(callbackHandle) + return err + } - // Wait for the async operation to complete. - err = <-done + // Wait for the async operation to complete. + err = <-done - return err + return err + }) } // SetPropertyString sets a string as property in the ten. This function has one @@ -622,29 +624,31 @@ func (p *tenEnv) SetPropertyString( ) } - // Create a channel to wait for the async operation in C to complete. - done := make(chan error, 1) - callbackHandle := newGoHandle(done) + return withCGO(func() error { + // Create a channel to wait for the async operation in C to complete. + done := make(chan error, 1) + callbackHandle := newGoHandle(done) - apiStatus := C.ten_go_ten_env_set_property_string( - p.cPtr, - unsafe.Pointer(unsafe.StringData(path)), - C.int(len(path)), - unsafe.Pointer(unsafe.StringData(value)), - C.int(len(value)), - C.uintptr_t(callbackHandle), - ) - err := withGoStatus(&apiStatus) - if err != nil { - // Clean up the handle if there was an error. - loadAndDeleteGoHandle(callbackHandle) - return err - } + apiStatus := C.ten_go_ten_env_set_property_string( + p.cPtr, + unsafe.Pointer(unsafe.StringData(path)), + C.int(len(path)), + unsafe.Pointer(unsafe.StringData(value)), + C.int(len(value)), + C.uintptr_t(callbackHandle), + ) + err := withGoStatus(&apiStatus) + if err != nil { + // Clean up the handle if there was an error. + loadAndDeleteGoHandle(callbackHandle) + return err + } - // Wait for the async operation to complete. - err = <-done + // Wait for the async operation to complete. + err = <-done - return err + return err + }) } // SetPropertyBytes sets a []byte as property in the ten. This function has one @@ -660,32 +664,34 @@ func (p *tenEnv) SetPropertyBytes( ) } - // Create a channel to wait for the async operation in C to complete. - done := make(chan error, 1) - callbackHandle := newGoHandle(done) + return withCGO(func() error { + // Create a channel to wait for the async operation in C to complete. + done := make(chan error, 1) + callbackHandle := newGoHandle(done) - apiStatus := C.ten_go_ten_env_set_property_buf( - p.cPtr, - unsafe.Pointer(unsafe.StringData(path)), - C.int(len(path)), - // Using either `unsafe.SliceData()` or `&value[0]` works fine. And - // `unsafe.SliceData()` reduces one memory allocation, while - // `runtime.convTslice` will be called if using `&value[0]`. - unsafe.Pointer(unsafe.SliceData(value)), - C.int(len(value)), - C.uintptr_t(callbackHandle), - ) - err := withGoStatus(&apiStatus) - if err != nil { - // Clean up the handle if there was an error. - loadAndDeleteGoHandle(callbackHandle) - return err - } + apiStatus := C.ten_go_ten_env_set_property_buf( + p.cPtr, + unsafe.Pointer(unsafe.StringData(path)), + C.int(len(path)), + // Using either `unsafe.SliceData()` or `&value[0]` works fine. And + // `unsafe.SliceData()` reduces one memory allocation, while + // `runtime.convTslice` will be called if using `&value[0]`. + unsafe.Pointer(unsafe.SliceData(value)), + C.int(len(value)), + C.uintptr_t(callbackHandle), + ) + err := withGoStatus(&apiStatus) + if err != nil { + // Clean up the handle if there was an error. + loadAndDeleteGoHandle(callbackHandle) + return err + } - // Wait for the async operation to complete. - err = <-done + // Wait for the async operation to complete. + err = <-done - return err + return err + }) } func (p *tenEnv) setPropertyFromJSONBytes(path string, value []byte) error { diff --git a/core/src/ten_runtime/binding/go/interface/ten/value.go b/core/src/ten_runtime/binding/go/interface/ten/value.go index e5c3cbda2..bea30f42e 100644 --- a/core/src/ten_runtime/binding/go/interface/ten/value.go +++ b/core/src/ten_runtime/binding/go/interface/ten/value.go @@ -240,108 +240,6 @@ func tenValueCreateFromJSON(jsonBytes []byte) (*value, error) { return res, nil } -func tenValueUnwrap( - cValue C.uintptr_t, - pt propType, - size uintptr, -) (any, error) { - switch pt { - case propTypeInt8: - return getPropInt8(func(v *C.int8_t) C.ten_go_status_t { - return C.ten_go_value_get_int8(cValue, v) - }) - - case propTypeInt16: - return getPropInt16(func(v *C.int16_t) C.ten_go_status_t { - return C.ten_go_value_get_int16(cValue, v) - }) - - case propTypeInt32: - return getPropInt32(func(v *C.int32_t) C.ten_go_status_t { - return C.ten_go_value_get_int32(cValue, v) - }) - - case propTypeInt64: - return getPropInt64(func(v *C.int64_t) C.ten_go_status_t { - return C.ten_go_value_get_int64(cValue, v) - }) - - case propTypeInt: - if is64bit { - return getPropInt64(func(v *C.int64_t) C.ten_go_status_t { - return C.ten_go_value_get_int64(cValue, v) - }) - } - return getPropInt32(func(v *C.int32_t) C.ten_go_status_t { - return C.ten_go_value_get_int32(cValue, v) - }) - - case propTypeUint8: - return getPropUint8(func(v *C.uint8_t) C.ten_go_status_t { - return C.ten_go_value_get_uint8(cValue, v) - }) - - case propTypeUint16: - return getPropUint16(func(t *C.uint16_t) C.ten_go_status_t { - return C.ten_go_value_get_uint16(cValue, t) - }) - - case propTypeUint32: - return getPropUint32(func(v *C.uint32_t) C.ten_go_status_t { - return C.ten_go_value_get_uint32(cValue, v) - }) - - case propTypeUint64: - return getPropUint64(func(v *C.uint64_t) C.ten_go_status_t { - return C.ten_go_value_get_uint64(cValue, v) - }) - - case propTypeUint: - if is64bit { - return getPropUint64(func(v *C.uint64_t) C.ten_go_status_t { - return C.ten_go_value_get_uint64(cValue, v) - }) - } - return getPropUint32(func(v *C.uint32_t) C.ten_go_status_t { - return C.ten_go_value_get_uint32(cValue, v) - }) - - case propTypeBool: - return getPropBool(func(v *C.bool) C.ten_go_status_t { - return C.ten_go_value_get_bool(cValue, v) - }) - - case propTypeFloat32: - return getPropFloat32(func(v *C.float) C.ten_go_status_t { - return C.ten_go_value_get_float32(cValue, v) - }) - - case propTypeFloat64: - return getPropFloat64(func(v *C.double) C.ten_go_status_t { - return C.ten_go_value_get_float64(cValue, v) - }) - - case propTypeStr: - return getPropStr(size, func(v unsafe.Pointer) C.ten_go_status_t { - return C.ten_go_value_get_string(cValue, v) - }) - - case propTypeBuf: - return getPropBytes(size, func(v unsafe.Pointer) C.ten_go_status_t { - return C.ten_go_value_get_buf(cValue, v) - }) - - case propTypePtr: - return getPropPtr(func(v *cHandle) C.ten_go_status_t { - return C.ten_go_value_get_ptr(cValue, v) - }) - - default: - // Should not happen. - return nil, newTenError(ErrnoInvalidType, "") - } -} - func tenValueDestroy(cValue C.uintptr_t) { C.ten_go_value_destroy(cValue) } diff --git a/tests/ten_runtime/integration/go/frequently_cgo_call_go/frequently_cgo_call_go_app/ten_packages/extension/extension_a/extension.go b/tests/ten_runtime/integration/go/frequently_cgo_call_go/frequently_cgo_call_go_app/ten_packages/extension/extension_a/extension.go index 720566677..214a4db74 100644 --- a/tests/ten_runtime/integration/go/frequently_cgo_call_go/frequently_cgo_call_go_app/ten_packages/extension/extension_a/extension.go +++ b/tests/ten_runtime/integration/go/frequently_cgo_call_go/frequently_cgo_call_go_app/ten_packages/extension/extension_a/extension.go @@ -49,6 +49,14 @@ func (p *extensionA) OnStart(tenEnv ten.TenEnv) { wg.Wait() tenEnv.OnStartDone() }() + + if err := tenEnv.SetPropertyString("empty_string", ""); err != nil { + panic("Should not happen") + } + + if em, err := tenEnv.GetPropertyString("empty_string"); err != nil || em != "" { + panic("Should not happen") + } } func (p *extensionA) OnCmd( @@ -90,6 +98,21 @@ func (p *extensionA) OnCmd( } <-done + // Set an empty string to cmd is permitted. + if err := cmdB.SetPropertyString("empty_string", ""); err != nil { + panic("Should not happen.") + } + + // Set an empty bytes to cmd is not permitted. + if err := cmdB.SetPropertyBytes("empty_bytes", []byte{}); err == nil { + panic("Should not happen.") + } + + some_bytes := []byte{1, 2, 3} + if err := cmdB.SetPropertyBytes("some_bytes", some_bytes); err != nil { + panic("Should not happen.") + } + tenEnv.SendCmd(cmdB, func(r ten.TenEnv, cs ten.CmdResult) { detail, err := cs.GetPropertyString("detail") if err != nil { diff --git a/tests/ten_runtime/integration/go/frequently_cgo_call_go/frequently_cgo_call_go_app/ten_packages/extension/extension_b/extension.go b/tests/ten_runtime/integration/go/frequently_cgo_call_go/frequently_cgo_call_go_app/ten_packages/extension/extension_b/extension.go index e758d2bbd..56c29fa42 100644 --- a/tests/ten_runtime/integration/go/frequently_cgo_call_go/frequently_cgo_call_go_app/ten_packages/extension/extension_b/extension.go +++ b/tests/ten_runtime/integration/go/frequently_cgo_call_go/frequently_cgo_call_go_app/ten_packages/extension/extension_b/extension.go @@ -40,6 +40,15 @@ func (p *extensionB) OnCmd( if cmdName == "B" { var count uint32 = 0 + // An empty string in cmd is permitted. + if em, err := cmd.GetPropertyString("empty_string"); err != nil || em != "" { + panic("Should not happen.") + } + + if em, err := cmd.GetPropertyBytes("some_bytes"); err != nil || len(em) != 3 { + panic("Should not happen.") + } + done := make(chan struct{}, 1) defer close(done)