From 4d5995dc46474095a1c35bb43a2b7c321b832edb Mon Sep 17 00:00:00 2001 From: Oseme Odigie Date: Mon, 29 Apr 2024 16:13:43 -0400 Subject: [PATCH 1/8] chore: added a comment that the NewSessionDispatch object should be allocated in C --- internal/ccsmp/ccsmp_core.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/ccsmp/ccsmp_core.go b/internal/ccsmp/ccsmp_core.go index b5c4166..c78b905 100644 --- a/internal/ccsmp/ccsmp_core.go +++ b/internal/ccsmp/ccsmp_core.go @@ -567,6 +567,7 @@ func NewSessionDispatch(id uint64) (*SolClientSessionRxMsgDispatchFuncInfo, uint // CGO defines void* as unsafe.Pointer, however it is just arbitrary data. // We want to store a number at void* ptr := uintptr(id) + // this function should be deprecated in favor allocating the dispatch struct on the C heap return &SolClientSessionRxMsgDispatchFuncInfo{ dispatchType: C.SOLCLIENT_DISPATCH_TYPE_CALLBACK, callback_p: (C.solClient_session_rxMsgCallbackFunc_t)(unsafe.Pointer(C.messageReceiveCallback)), From aa36a01e494bc5f392093b4665b8aa7071d3d2fe Mon Sep 17 00:00:00 2001 From: Oseme Odigie Date: Tue, 30 Apr 2024 15:55:42 -0400 Subject: [PATCH 2/8] SOL-117431: replaced all usages of the C.uintptr_to_void_p method with direct casts to void* in C --- internal/ccsmp/ccsmp_core.go | 37 ++++++----- internal/ccsmp/ccsmp_flow.go | 23 +++++-- internal/ccsmp/ccsmp_helper.c | 115 ++++++++++++++++++++++++---------- internal/ccsmp/ccsmp_helper.h | 67 +++++++++++++------- 4 files changed, 166 insertions(+), 76 deletions(-) diff --git a/internal/ccsmp/ccsmp_core.go b/internal/ccsmp/ccsmp_core.go index c78b905..c8d567b 100644 --- a/internal/ccsmp/ccsmp_core.go +++ b/internal/ccsmp/ccsmp_core.go @@ -297,9 +297,9 @@ func (context *SolClientContext) SolClientSessionCreate(properties []string) (se var sessionFuncInfo C.solClient_session_createFuncInfo_t sessionFuncInfo.rxMsgInfo.callback_p = (C.solClient_session_rxMsgCallbackFunc_t)(unsafe.Pointer(C.defaultMessageReceiveCallback)) - sessionFuncInfo.rxMsgInfo.user_p = nil + sessionFuncInfo.rxMsgInfo.user_p = nil // <-- if this should be set; it should be done in C (the ccsmp_helper.c) sessionFuncInfo.eventInfo.callback_p = (C.solClient_session_eventCallbackFunc_t)(unsafe.Pointer(C.eventCallback)) - sessionFuncInfo.eventInfo.user_p = nil + sessionFuncInfo.eventInfo.user_p = nil // <-- if this should be set; it should be done in C (the ccsmp_helper.c) solClientErrorInfo := handleCcsmpError(func() SolClientReturnCode { return C.solClient_session_create(sessionPropsP, context.pointer, &sessionP, &sessionFuncInfo, (C.size_t)(unsafe.Sizeof(sessionFuncInfo))) @@ -348,12 +348,12 @@ func (session *SolClientSession) solClientSessionSubscribeWithFlags(topic string return handleCcsmpError(func() SolClientReturnCode { cString := C.CString(topic) defer C.free(unsafe.Pointer(cString)) - // This is not an unsafe usage of unsafe.Pointer as we are using correlationId as data, not as a pointer + // This is not an unsafe usage of unsafe.Pointer as we are using dispatchID and correlationID as data, not as pointers return C.SessionTopicSubscribeWithFlags(session.pointer, cString, flags, - C.uintptr_to_void_p(C.solClient_uint64_t(dispatchID)), - C.uintptr_to_void_p(C.solClient_uint64_t(correlationID))) + C.solClient_uint64_t(dispatchID), + C.solClient_uint64_t(correlationID)) }) } @@ -362,12 +362,12 @@ func (session *SolClientSession) solClientSessionSubscribeReplyTopicWithFlags(to return handleCcsmpError(func() SolClientReturnCode { cString := C.CString(topic) defer C.free(unsafe.Pointer(cString)) - // This is not an unsafe usage of unsafe.Pointer as we are using correlationId as data, not as a pointer + // This is not an unsafe usage of unsafe.Pointer as we are using dispatchID and correlationID as data, not as pointers return C.SessionReplyTopicSubscribeWithFlags(session.pointer, cString, flags, - C.uintptr_to_void_p(C.solClient_uint64_t(dispatchID)), - C.uintptr_to_void_p(C.solClient_uint64_t(correlationID))) + C.solClient_uint64_t(dispatchID), + C.solClient_uint64_t(correlationID)) }) } @@ -376,12 +376,12 @@ func (session *SolClientSession) solClientSessionUnsubscribeWithFlags(topic stri return handleCcsmpError(func() SolClientReturnCode { cString := C.CString(topic) defer C.free(unsafe.Pointer(cString)) - // This is not an unsafe usage of unsafe.Pointer as we are using correlationId as data, not as a pointer + // This is not an unsafe usage of unsafe.Pointer as we are using dispatchID and correlationID as data, not as pointers return C.SessionTopicUnsubscribeWithFlags(session.pointer, cString, flags, - C.uintptr_to_void_p(C.solClient_uint64_t(dispatchID)), - C.uintptr_to_void_p(C.solClient_uint64_t(correlationID))) + C.solClient_uint64_t(dispatchID), + C.solClient_uint64_t(correlationID)) }) } @@ -390,12 +390,12 @@ func (session *SolClientSession) solClientSessionUnsubscribeReplyTopicWithFlags( return handleCcsmpError(func() SolClientReturnCode { cString := C.CString(topic) defer C.free(unsafe.Pointer(cString)) - // This is not an unsafe usage of unsafe.Pointer as we are using correlationId as data, not as a pointer + // This is not an unsafe usage of unsafe.Pointer as we are using dispatchID and correlationID as data, not as pointers return C.SessionReplyTopicUnsubscribeWithFlags(session.pointer, cString, flags, - C.uintptr_to_void_p(C.solClient_uint64_t(dispatchID)), - C.uintptr_to_void_p(C.solClient_uint64_t(correlationID))) + C.solClient_uint64_t(dispatchID), + C.solClient_uint64_t(correlationID)) }) } @@ -436,8 +436,11 @@ func (session *SolClientSession) SolClientEndpointUnsusbcribe(properties []strin endpointProps, endpointFree := ToCArray(properties, true) defer endpointFree() // This is not an unsafe usage of unsafe.Pointer as we are using correlationId as data, not as a pointer - return C.solClient_session_endpointTopicUnsubscribe(endpointProps, session.pointer, - C.SOLCLIENT_SUBSCRIBE_FLAGS_REQUEST_CONFIRM, cString, C.uintptr_to_void_p(C.solClient_uint64_t(correlationID))) + return C.SessionTopicEndpointUnsubscribeWithFlags(session.pointer, + endpointProps, + C.SOLCLIENT_SUBSCRIBE_FLAGS_REQUEST_CONFIRM, + cString, + C.solClient_uint64_t(correlationID)) }) } @@ -571,7 +574,7 @@ func NewSessionDispatch(id uint64) (*SolClientSessionRxMsgDispatchFuncInfo, uint return &SolClientSessionRxMsgDispatchFuncInfo{ dispatchType: C.SOLCLIENT_DISPATCH_TYPE_CALLBACK, callback_p: (C.solClient_session_rxMsgCallbackFunc_t)(unsafe.Pointer(C.messageReceiveCallback)), - user_p: C.uintptr_to_void_p(C.solClient_uint64_t(ptr)), + user_p: nil, // <-- if this should be set; it should be done in C (the ccsmp_helper.c) rfu_p: nil, }, ptr } diff --git a/internal/ccsmp/ccsmp_flow.go b/internal/ccsmp/ccsmp_flow.go index ddf8ff0..36d17e6 100644 --- a/internal/ccsmp/ccsmp_flow.go +++ b/internal/ccsmp/ccsmp_flow.go @@ -102,9 +102,9 @@ func (session *SolClientSession) SolClientSessionCreateFlow(properties []string, // These are not a misuse of unsafe.Pointer as the value is used for correlation flowCreateFuncInfo.rxMsgInfo.callback_p = (C.solClient_flow_rxMsgCallbackFunc_t)(unsafe.Pointer(C.flowMessageReceiveCallback)) - flowCreateFuncInfo.rxMsgInfo.user_p = C.uintptr_to_void_p(C.solClient_uint64_t(flowID)) + flowCreateFuncInfo.rxMsgInfo.user_p = nil // <-- this will be set in the C.SessionFlowCreate() helper function (the ccsmp_helper.c) flowCreateFuncInfo.eventInfo.callback_p = (C.solClient_flow_eventCallbackFunc_t)(unsafe.Pointer(C.flowEventCallback)) - flowCreateFuncInfo.eventInfo.user_p = C.uintptr_to_void_p(C.solClient_uint64_t(flowID)) + flowCreateFuncInfo.eventInfo.user_p = nil // <-- this will be set in the C.SessionFlowCreate() helper function (the ccsmp_helper.c) flowToRXCallbackMap.Store(flowID, msgCallback) flowToEventCallbackMap.Store(flowID, eventCallback) @@ -112,7 +112,12 @@ func (session *SolClientSession) SolClientSessionCreateFlow(properties []string, flow := &SolClientFlow{} flow.userP = flowID err := handleCcsmpError(func() SolClientReturnCode { - return C.solClient_session_createFlow(flowPropsP, session.pointer, &flow.pointer, &flowCreateFuncInfo, (C.size_t)(unsafe.Sizeof(flowCreateFuncInfo))) + // This is not an unsafe usage of unsafe.Pointer as we are using flowID as data, not as a pointer + return C.SessionFlowCreate(session.pointer, + flowPropsP, + &flow.pointer, + &flowCreateFuncInfo, + C.solClient_uint64_t(flowID)) }) if err != nil { return nil, err @@ -154,7 +159,11 @@ func (flow *SolClientFlow) SolClientFlowSubscribe(topic string, correlationID ui cString := C.CString(topic) defer C.free(unsafe.Pointer(cString)) // This is not an unsafe usage of unsafe.Pointer as we are using correlationId as data, not as a pointer - return C.solClient_flow_topicSubscribeWithDispatch(flow.pointer, C.SOLCLIENT_SUBSCRIBE_FLAGS_REQUEST_CONFIRM, cString, nil, C.uintptr_to_void_p(C.solClient_uint64_t(correlationID))) + return C.FlowTopicSubscribeWithDispatch(flow.pointer, + C.SOLCLIENT_SUBSCRIBE_FLAGS_REQUEST_CONFIRM, + cString, + nil, + C.solClient_uint64_t(correlationID)) }) } @@ -164,7 +173,11 @@ func (flow *SolClientFlow) SolClientFlowUnsubscribe(topic string, correlationID cString := C.CString(topic) defer C.free(unsafe.Pointer(cString)) // This is not an unsafe usage of unsafe.Pointer as we are using correlationId as data, not as a pointer - return C.solClient_flow_topicUnsubscribeWithDispatch(flow.pointer, C.SOLCLIENT_SUBSCRIBE_FLAGS_REQUEST_CONFIRM, cString, nil, C.uintptr_to_void_p(C.solClient_uint64_t(correlationID))) + return C.FlowTopicUnsubscribeWithDispatch(flow.pointer, + C.SOLCLIENT_SUBSCRIBE_FLAGS_REQUEST_CONFIRM, + cString, + nil, + C.solClient_uint64_t(correlationID)) }) } diff --git a/internal/ccsmp/ccsmp_helper.c b/internal/ccsmp/ccsmp_helper.c index 35ba65c..157aea3 100644 --- a/internal/ccsmp/ccsmp_helper.c +++ b/internal/ccsmp/ccsmp_helper.c @@ -27,11 +27,6 @@ messageReceiveCallback(solClient_opaqueSession_pt opaqueSession_p, solClient_opa solClient_rxMsgCallback_returnCode_t requestResponseReplyMessageReceiveCallback(solClient_opaqueSession_pt opaqueSession_p, solClient_opaqueMsg_pt msg_p, void *user_p); -void *uintptr_to_void_p(solClient_uint64_t ptr) -{ - return (void *)ptr; -} - solClient_returnCode_t solClientgo_msg_isRequestReponseMsg(solClient_opaqueMsg_pt msg_p, char **correlationId_p) { solClient_returnCode_t rc = SOLCLIENT_FAIL; @@ -54,54 +49,95 @@ solClientgo_msg_isRequestReponseMsg(solClient_opaqueMsg_pt msg_p, char **correla } solClient_returnCode_t -_SessionTopicSubscribeWithFlags( solClient_opaqueSession_pt opaqueSession_p, - const char *topicSubscription_p, +SessionFlowCreate( solClient_opaqueSession_pt opaqueSession_p, + solClient_propertyArray_pt flowPropsP, + solClient_opaqueFlow_pt *opaqueFlow_p, + solClient_flow_createFuncInfo_t *funcInfo_p, + solClient_uint64_t flowID) +{ + /* set the flowID in the flow create struct */ + funcInfo_p->rxMsgInfo.user_p = (void *) flowID; + funcInfo_p->eventInfo.user_p = (void *) flowID; + + return solClient_session_createFlow(flowPropsP, opaqueSession_p, opaqueFlow_p, funcInfo_p, sizeof(*funcInfo_p)); +} + +solClient_returnCode_t +FlowTopicSubscribeWithDispatch( solClient_opaqueFlow_pt opaqueFlow_p, solClient_subscribeFlags_t flags, - solClient_session_rxMsgCallbackFunc_t callback_p, - void *dispatchId_p, - void *correlationTag_p) + const char *topicSubscription_p, + solClient_flow_rxMsgDispatchFuncInfo_t *dispatchFuncInfo_p, + solClient_uint64_t correlationTag) +{ + return solClient_flow_topicSubscribeWithDispatch( opaqueFlow_p, + flags, + topicSubscription_p, + dispatchFuncInfo_p, + (void *)correlationTag); +} + +solClient_returnCode_t +FlowTopicUnsubscribeWithDispatch( solClient_opaqueFlow_pt opaqueFlow_p, + solClient_subscribeFlags_t flags, + const char *topicSubscription_p, + solClient_flow_rxMsgDispatchFuncInfo_t *dispatchFuncInfo_p, + solClient_uint64_t correlationTag) +{ + return solClient_flow_topicUnsubscribeWithDispatch( opaqueFlow_p, + flags, + topicSubscription_p, + dispatchFuncInfo_p, + (void *)correlationTag); +} + +solClient_returnCode_t +_SessionTopicSubscribeWithFlags( solClient_opaqueSession_pt opaqueSession_p, + const char *topicSubscription_p, + solClient_subscribeFlags_t flags, + solClient_session_rxMsgCallbackFunc_t callback_p, + solClient_uint64_t dispatchId, + solClient_uint64_t correlationTag) { solClient_session_rxMsgDispatchFuncInfo_t dispatchInfo; /* msg dispatch callback to set */ dispatchInfo.dispatchType = SOLCLIENT_DISPATCH_TYPE_CALLBACK; dispatchInfo.callback_p = callback_p; - dispatchInfo.user_p = dispatchId_p; + dispatchInfo.user_p = (void *)dispatchId; dispatchInfo.rfu_p = NULL; return solClient_session_topicSubscribeWithDispatch ( opaqueSession_p, flags, topicSubscription_p, &dispatchInfo, - correlationTag_p); + (void *)correlationTag); } - solClient_returnCode_t SessionTopicSubscribeWithFlags( solClient_opaqueSession_pt opaqueSession_p, const char *topicSubscription_p, solClient_subscribeFlags_t flags, - void *dispatchId_p, - void *correlationTag_p) + solClient_uint64_t dispatchId, + solClient_uint64_t correlationTag) { return _SessionTopicSubscribeWithFlags ( opaqueSession_p, topicSubscription_p, flags, messageReceiveCallback, - dispatchId_p, - correlationTag_p ); + dispatchId, + correlationTag ); } solClient_returnCode_t SessionReplyTopicSubscribeWithFlags( solClient_opaqueSession_pt opaqueSession_p, const char *topicSubscription_p, solClient_subscribeFlags_t flags, - void *dispatchId_p, - void *correlationTag_p) + solClient_uint64_t dispatchId, + solClient_uint64_t correlationTag) { return _SessionTopicSubscribeWithFlags ( opaqueSession_p, topicSubscription_p, flags, requestResponseReplyMessageReceiveCallback, - dispatchId_p, - correlationTag_p ); + dispatchId, + correlationTag ); } solClient_returnCode_t @@ -109,48 +145,61 @@ _SessionTopicUnsubscribeWithFlags( solClient_opaqueSession_pt opaqueSession_p, const char *topicSubscription_p, solClient_subscribeFlags_t flags, solClient_session_rxMsgCallbackFunc_t callback_p, - void *dispatchId_p, - void *correlationTag_p) + solClient_uint64_t dispatchId, + solClient_uint64_t correlationTag) { solClient_session_rxMsgDispatchFuncInfo_t dispatchInfo; /* msg dispatch callback to set */ dispatchInfo.dispatchType = SOLCLIENT_DISPATCH_TYPE_CALLBACK; dispatchInfo.callback_p = callback_p; - dispatchInfo.user_p = dispatchId_p; + dispatchInfo.user_p = (void *)dispatchId; dispatchInfo.rfu_p = NULL; return solClient_session_topicUnsubscribeWithDispatch ( opaqueSession_p, flags, topicSubscription_p, &dispatchInfo, - correlationTag_p); + (void *)correlationTag); } solClient_returnCode_t SessionTopicUnsubscribeWithFlags( solClient_opaqueSession_pt opaqueSession_p, const char *topicSubscription_p, solClient_subscribeFlags_t flags, - void *dispatchId_p, - void *correlationTag_p) + solClient_uint64_t dispatchId, + solClient_uint64_t correlationTag) { return _SessionTopicUnsubscribeWithFlags ( opaqueSession_p, topicSubscription_p, flags, messageReceiveCallback, - dispatchId_p, - correlationTag_p ); + dispatchId, + correlationTag ); } solClient_returnCode_t SessionReplyTopicUnsubscribeWithFlags( solClient_opaqueSession_pt opaqueSession_p, const char *topicSubscription_p, solClient_subscribeFlags_t flags, - void *dispatchId_p, - void *correlationTag_p) + solClient_uint64_t dispatchId, + solClient_uint64_t correlationTag) { return _SessionTopicUnsubscribeWithFlags ( opaqueSession_p, topicSubscription_p, flags, requestResponseReplyMessageReceiveCallback, - dispatchId_p, - correlationTag_p ); + dispatchId, + correlationTag ); } +solClient_returnCode_t +SessionTopicEndpointUnsubscribeWithFlags( solClient_opaqueSession_pt opaqueSession_p, + solClient_propertyArray_pt endpointProps, + solClient_subscribeFlags_t flags, + const char *topicSubscription_p, + solClient_uint64_t correlationTag) +{ + return solClient_session_endpointTopicUnsubscribe( endpointProps, + opaqueSession_p, + flags, + topicSubscription_p, + (void *)correlationTag); +} diff --git a/internal/ccsmp/ccsmp_helper.h b/internal/ccsmp/ccsmp_helper.h index 1aeaf9e..9cd387c 100644 --- a/internal/ccsmp/ccsmp_helper.h +++ b/internal/ccsmp/ccsmp_helper.h @@ -46,36 +46,61 @@ typedef struct solClient_errorInfo_wrapper * operating systems are supported, this may need to change to a more complex * definition. */ -void * -uintptr_to_void_p(solClient_uint64_t ptr); +solClient_returnCode_t SessionFlowCreate( + solClient_opaqueSession_pt opaqueSession_p, + solClient_propertyArray_pt flowPropsP, + solClient_opaqueFlow_pt *opaqueFlow_p, + solClient_flow_createFuncInfo_t *funcInfo_p, + solClient_uint64_t flowID_p); -solClient_returnCode_t SessionTopicSubscribeWithFlags( - solClient_opaqueSession_pt opaqueSession_p, - const char *topicSubscription_p, +solClient_returnCode_t FlowTopicSubscribeWithDispatch( + solClient_opaqueFlow_pt opaqueFlow_p, solClient_subscribeFlags_t flags, - void *dispatchId_p, - void *correlationTag_p); + const char *topicSubscription_p, + solClient_flow_rxMsgDispatchFuncInfo_t *dispatchFuncInfo_p, + solClient_uint64_t correlationTag); -solClient_returnCode_t SessionTopicUnsubscribeWithFlags( - solClient_opaqueSession_pt opaqueSession_p, - const char *topicSubscription_p, +solClient_returnCode_t FlowTopicUnsubscribeWithDispatch( + solClient_opaqueFlow_pt opaqueFlow_p, solClient_subscribeFlags_t flags, - void *dispatchId_p, - void *correlationTag_p); + const char *topicSubscription_p, + solClient_flow_rxMsgDispatchFuncInfo_t *dispatchFuncInfo_p, + solClient_uint64_t correlationTag); + +solClient_returnCode_t SessionTopicSubscribeWithFlags( + solClient_opaqueSession_pt opaqueSession_p, + const char *topicSubscription_p, + solClient_subscribeFlags_t flags, + solClient_uint64_t dispatchId, + solClient_uint64_t correlationTag); + +solClient_returnCode_t SessionTopicUnsubscribeWithFlags( + solClient_opaqueSession_pt opaqueSession_p, + const char *topicSubscription_p, + solClient_subscribeFlags_t flags, + solClient_uint64_t dispatchId, + solClient_uint64_t correlationTag); solClient_returnCode_t SessionReplyTopicSubscribeWithFlags( - solClient_opaqueSession_pt opaqueSession_p, - const char *topicSubscription_p, - solClient_subscribeFlags_t flags, - void *dispatchId_p, - void *correlationTag_p); + solClient_opaqueSession_pt opaqueSession_p, + const char *topicSubscription_p, + solClient_subscribeFlags_t flags, + solClient_uint64_t dispatchId, + solClient_uint64_t correlationTag); solClient_returnCode_t SessionReplyTopicUnsubscribeWithFlags( - solClient_opaqueSession_pt opaqueSession_p, - const char *topicSubscription_p, + solClient_opaqueSession_pt opaqueSession_p, + const char *topicSubscription_p, + solClient_subscribeFlags_t flags, + solClient_uint64_t dispatchId, + solClient_uint64_t correlationTag); + +solClient_returnCode_t SessionTopicEndpointUnsubscribeWithFlags( + solClient_opaqueSession_pt opaqueSession_p, + solClient_propertyArray_pt endpointProps, solClient_subscribeFlags_t flags, - void *dispatchId_p, - void *correlationTag_p); + const char *topicSubscription_p, + solClient_uint64_t correlationTag); /** * Definition of solclientgo correlation prefix From a4279024e9b80f180ad02638075247bb7472485f Mon Sep 17 00:00:00 2001 From: Oseme Odigie Date: Thu, 2 May 2024 10:06:45 -0400 Subject: [PATCH 3/8] feat: responded to the requested changes on the PR --- internal/ccsmp/ccsmp_core.go | 12 +++--------- internal/ccsmp/ccsmp_flow.go | 12 +----------- internal/ccsmp/ccsmp_helper.c | 19 +++++++++++++------ internal/ccsmp/ccsmp_helper.h | 2 +- 4 files changed, 18 insertions(+), 27 deletions(-) diff --git a/internal/ccsmp/ccsmp_core.go b/internal/ccsmp/ccsmp_core.go index c8d567b..461689d 100644 --- a/internal/ccsmp/ccsmp_core.go +++ b/internal/ccsmp/ccsmp_core.go @@ -297,9 +297,9 @@ func (context *SolClientContext) SolClientSessionCreate(properties []string) (se var sessionFuncInfo C.solClient_session_createFuncInfo_t sessionFuncInfo.rxMsgInfo.callback_p = (C.solClient_session_rxMsgCallbackFunc_t)(unsafe.Pointer(C.defaultMessageReceiveCallback)) - sessionFuncInfo.rxMsgInfo.user_p = nil // <-- if this should be set; it should be done in C (the ccsmp_helper.c) + sessionFuncInfo.rxMsgInfo.user_p = nil sessionFuncInfo.eventInfo.callback_p = (C.solClient_session_eventCallbackFunc_t)(unsafe.Pointer(C.eventCallback)) - sessionFuncInfo.eventInfo.user_p = nil // <-- if this should be set; it should be done in C (the ccsmp_helper.c) + sessionFuncInfo.eventInfo.user_p = nil solClientErrorInfo := handleCcsmpError(func() SolClientReturnCode { return C.solClient_session_create(sessionPropsP, context.pointer, &sessionP, &sessionFuncInfo, (C.size_t)(unsafe.Sizeof(sessionFuncInfo))) @@ -348,7 +348,6 @@ func (session *SolClientSession) solClientSessionSubscribeWithFlags(topic string return handleCcsmpError(func() SolClientReturnCode { cString := C.CString(topic) defer C.free(unsafe.Pointer(cString)) - // This is not an unsafe usage of unsafe.Pointer as we are using dispatchID and correlationID as data, not as pointers return C.SessionTopicSubscribeWithFlags(session.pointer, cString, flags, @@ -362,7 +361,6 @@ func (session *SolClientSession) solClientSessionSubscribeReplyTopicWithFlags(to return handleCcsmpError(func() SolClientReturnCode { cString := C.CString(topic) defer C.free(unsafe.Pointer(cString)) - // This is not an unsafe usage of unsafe.Pointer as we are using dispatchID and correlationID as data, not as pointers return C.SessionReplyTopicSubscribeWithFlags(session.pointer, cString, flags, @@ -376,7 +374,6 @@ func (session *SolClientSession) solClientSessionUnsubscribeWithFlags(topic stri return handleCcsmpError(func() SolClientReturnCode { cString := C.CString(topic) defer C.free(unsafe.Pointer(cString)) - // This is not an unsafe usage of unsafe.Pointer as we are using dispatchID and correlationID as data, not as pointers return C.SessionTopicUnsubscribeWithFlags(session.pointer, cString, flags, @@ -390,7 +387,6 @@ func (session *SolClientSession) solClientSessionUnsubscribeReplyTopicWithFlags( return handleCcsmpError(func() SolClientReturnCode { cString := C.CString(topic) defer C.free(unsafe.Pointer(cString)) - // This is not an unsafe usage of unsafe.Pointer as we are using dispatchID and correlationID as data, not as pointers return C.SessionReplyTopicUnsubscribeWithFlags(session.pointer, cString, flags, @@ -435,7 +431,6 @@ func (session *SolClientSession) SolClientEndpointUnsusbcribe(properties []strin defer C.free(unsafe.Pointer(cString)) endpointProps, endpointFree := ToCArray(properties, true) defer endpointFree() - // This is not an unsafe usage of unsafe.Pointer as we are using correlationId as data, not as a pointer return C.SessionTopicEndpointUnsubscribeWithFlags(session.pointer, endpointProps, C.SOLCLIENT_SUBSCRIBE_FLAGS_REQUEST_CONFIRM, @@ -570,11 +565,10 @@ func NewSessionDispatch(id uint64) (*SolClientSessionRxMsgDispatchFuncInfo, uint // CGO defines void* as unsafe.Pointer, however it is just arbitrary data. // We want to store a number at void* ptr := uintptr(id) - // this function should be deprecated in favor allocating the dispatch struct on the C heap return &SolClientSessionRxMsgDispatchFuncInfo{ dispatchType: C.SOLCLIENT_DISPATCH_TYPE_CALLBACK, callback_p: (C.solClient_session_rxMsgCallbackFunc_t)(unsafe.Pointer(C.messageReceiveCallback)), - user_p: nil, // <-- if this should be set; it should be done in C (the ccsmp_helper.c) + user_p: nil, // this should be set to the uintptr rfu_p: nil, }, ptr } diff --git a/internal/ccsmp/ccsmp_flow.go b/internal/ccsmp/ccsmp_flow.go index 36d17e6..0e019ad 100644 --- a/internal/ccsmp/ccsmp_flow.go +++ b/internal/ccsmp/ccsmp_flow.go @@ -96,23 +96,15 @@ func (session *SolClientSession) SolClientSessionCreateFlow(properties []string, flowPropsP, sessionPropertiesFreeFunction := ToCArray(properties, true) defer sessionPropertiesFreeFunction() - var flowCreateFuncInfo C.solClient_flow_createFuncInfo_t - flowID := atomic.AddUintptr(&flowID, 1) - // These are not a misuse of unsafe.Pointer as the value is used for correlation - flowCreateFuncInfo.rxMsgInfo.callback_p = (C.solClient_flow_rxMsgCallbackFunc_t)(unsafe.Pointer(C.flowMessageReceiveCallback)) - flowCreateFuncInfo.rxMsgInfo.user_p = nil // <-- this will be set in the C.SessionFlowCreate() helper function (the ccsmp_helper.c) - flowCreateFuncInfo.eventInfo.callback_p = (C.solClient_flow_eventCallbackFunc_t)(unsafe.Pointer(C.flowEventCallback)) - flowCreateFuncInfo.eventInfo.user_p = nil // <-- this will be set in the C.SessionFlowCreate() helper function (the ccsmp_helper.c) - flowToRXCallbackMap.Store(flowID, msgCallback) flowToEventCallbackMap.Store(flowID, eventCallback) flow := &SolClientFlow{} flow.userP = flowID err := handleCcsmpError(func() SolClientReturnCode { - // This is not an unsafe usage of unsafe.Pointer as we are using flowID as data, not as a pointer + var flowCreateFuncInfo C.solClient_flow_createFuncInfo_t return C.SessionFlowCreate(session.pointer, flowPropsP, &flow.pointer, @@ -158,7 +150,6 @@ func (flow *SolClientFlow) SolClientFlowSubscribe(topic string, correlationID ui return handleCcsmpError(func() SolClientReturnCode { cString := C.CString(topic) defer C.free(unsafe.Pointer(cString)) - // This is not an unsafe usage of unsafe.Pointer as we are using correlationId as data, not as a pointer return C.FlowTopicSubscribeWithDispatch(flow.pointer, C.SOLCLIENT_SUBSCRIBE_FLAGS_REQUEST_CONFIRM, cString, @@ -172,7 +163,6 @@ func (flow *SolClientFlow) SolClientFlowUnsubscribe(topic string, correlationID return handleCcsmpError(func() SolClientReturnCode { cString := C.CString(topic) defer C.free(unsafe.Pointer(cString)) - // This is not an unsafe usage of unsafe.Pointer as we are using correlationId as data, not as a pointer return C.FlowTopicUnsubscribeWithDispatch(flow.pointer, C.SOLCLIENT_SUBSCRIBE_FLAGS_REQUEST_CONFIRM, cString, diff --git a/internal/ccsmp/ccsmp_helper.c b/internal/ccsmp/ccsmp_helper.c index 157aea3..9f100e6 100644 --- a/internal/ccsmp/ccsmp_helper.c +++ b/internal/ccsmp/ccsmp_helper.c @@ -27,6 +27,11 @@ messageReceiveCallback(solClient_opaqueSession_pt opaqueSession_p, solClient_opa solClient_rxMsgCallback_returnCode_t requestResponseReplyMessageReceiveCallback(solClient_opaqueSession_pt opaqueSession_p, solClient_opaqueMsg_pt msg_p, void *user_p); +solClient_rxMsgCallback_returnCode_t +flowMessageReceiveCallback(solClient_opaqueFlow_pt opaqueFlow_p, solClient_opaqueMsg_pt msg_p, void *user_p); + +void flowEventCallback(solClient_opaqueFlow_pt opaqueFlow_p, solClient_flow_eventCallbackInfo_pt eventInfo_p, void *user_p); + solClient_returnCode_t solClientgo_msg_isRequestReponseMsg(solClient_opaqueMsg_pt msg_p, char **correlationId_p) { solClient_returnCode_t rc = SOLCLIENT_FAIL; @@ -51,15 +56,17 @@ solClientgo_msg_isRequestReponseMsg(solClient_opaqueMsg_pt msg_p, char **correla solClient_returnCode_t SessionFlowCreate( solClient_opaqueSession_pt opaqueSession_p, solClient_propertyArray_pt flowPropsP, - solClient_opaqueFlow_pt *opaqueFlow_p, - solClient_flow_createFuncInfo_t *funcInfo_p, - solClient_uint64_t flowID) + solClient_opaqueFlow_pt *opaqueFlow_p, + solClient_flow_createFuncInfo_t *flowCreateFuncInfo, + solClient_uint64_t flowID) { /* set the flowID in the flow create struct */ - funcInfo_p->rxMsgInfo.user_p = (void *) flowID; - funcInfo_p->eventInfo.user_p = (void *) flowID; + flowCreateFuncInfo->rxMsgInfo.user_p = (void *)flowID; + flowCreateFuncInfo->eventInfo.user_p = (void *)flowID; + flowCreateFuncInfo->rxMsgInfo.callback_p = flowMessageReceiveCallback; + flowCreateFuncInfo->eventInfo.callback_p = (solClient_flow_eventCallbackFunc_t)flowEventCallback; - return solClient_session_createFlow(flowPropsP, opaqueSession_p, opaqueFlow_p, funcInfo_p, sizeof(*funcInfo_p)); + return solClient_session_createFlow(flowPropsP, opaqueSession_p, opaqueFlow_p, flowCreateFuncInfo, sizeof(*flowCreateFuncInfo)); } solClient_returnCode_t diff --git a/internal/ccsmp/ccsmp_helper.h b/internal/ccsmp/ccsmp_helper.h index 9cd387c..356d94c 100644 --- a/internal/ccsmp/ccsmp_helper.h +++ b/internal/ccsmp/ccsmp_helper.h @@ -50,7 +50,7 @@ solClient_returnCode_t SessionFlowCreate( solClient_opaqueSession_pt opaqueSession_p, solClient_propertyArray_pt flowPropsP, solClient_opaqueFlow_pt *opaqueFlow_p, - solClient_flow_createFuncInfo_t *funcInfo_p, + solClient_flow_createFuncInfo_t *flowCreateFuncInfo, solClient_uint64_t flowID_p); solClient_returnCode_t FlowTopicSubscribeWithDispatch( From 72cfe36bfa6fdc450a26e947ffb5b9bec785ac0f Mon Sep 17 00:00:00 2001 From: Chris Morgan <17009318+cjwmorgan-sol@users.noreply.github.com> Date: Thu, 2 May 2024 11:42:49 -0400 Subject: [PATCH 4/8] SOL-117746: Merge main history to dev branch after v1.6.0 release Version bump to 1.6.1 Signed-off-by: Chris Morgan <17009318+cjwmorgan-sol@users.noreply.github.com> --- version.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/version.go b/version.go index 1b6edb3..0fce878 100644 --- a/version.go +++ b/version.go @@ -23,4 +23,4 @@ func init() { core.SetVersion(version) } -const version = "1.6.0" +const version = "1.6.1" From a3dfcb2485f450df58f77dc2a120da40dbb5c31e Mon Sep 17 00:00:00 2001 From: Oseme Odigie Date: Thu, 2 May 2024 11:45:46 -0400 Subject: [PATCH 5/8] feat: allocated the other properties of the solClient_flow_createFuncInfo_t to ensure we do not get a segfault --- internal/ccsmp/ccsmp_flow.go | 6 +----- internal/ccsmp/ccsmp_helper.c | 17 ++++++++++------- internal/ccsmp/ccsmp_helper.h | 1 - 3 files changed, 11 insertions(+), 13 deletions(-) diff --git a/internal/ccsmp/ccsmp_flow.go b/internal/ccsmp/ccsmp_flow.go index 0e019ad..c482e33 100644 --- a/internal/ccsmp/ccsmp_flow.go +++ b/internal/ccsmp/ccsmp_flow.go @@ -33,9 +33,6 @@ import ( #include "solclient/solClient.h" #include "solclient/solClientMsg.h" #include "./ccsmp_helper.h" - -solClient_rxMsgCallback_returnCode_t flowMessageReceiveCallback ( solClient_opaqueFlow_pt opaqueFlow_p, solClient_opaqueMsg_pt msg_p, void *user_p ); -void flowEventCallback ( solClient_opaqueFlow_pt opaqueFlow_p, solClient_flow_eventCallbackInfo_pt eventInfo_p, void *user_p ); */ import "C" @@ -104,11 +101,10 @@ func (session *SolClientSession) SolClientSessionCreateFlow(properties []string, flow := &SolClientFlow{} flow.userP = flowID err := handleCcsmpError(func() SolClientReturnCode { - var flowCreateFuncInfo C.solClient_flow_createFuncInfo_t + // this will register the goFlowMessageReceiveCallback and goFlowEventCallback callbacks with the flowID return C.SessionFlowCreate(session.pointer, flowPropsP, &flow.pointer, - &flowCreateFuncInfo, C.solClient_uint64_t(flowID)) }) if err != nil { diff --git a/internal/ccsmp/ccsmp_helper.c b/internal/ccsmp/ccsmp_helper.c index 9f100e6..c6d93d1 100644 --- a/internal/ccsmp/ccsmp_helper.c +++ b/internal/ccsmp/ccsmp_helper.c @@ -57,16 +57,19 @@ solClient_returnCode_t SessionFlowCreate( solClient_opaqueSession_pt opaqueSession_p, solClient_propertyArray_pt flowPropsP, solClient_opaqueFlow_pt *opaqueFlow_p, - solClient_flow_createFuncInfo_t *flowCreateFuncInfo, solClient_uint64_t flowID) { /* set the flowID in the flow create struct */ - flowCreateFuncInfo->rxMsgInfo.user_p = (void *)flowID; - flowCreateFuncInfo->eventInfo.user_p = (void *)flowID; - flowCreateFuncInfo->rxMsgInfo.callback_p = flowMessageReceiveCallback; - flowCreateFuncInfo->eventInfo.callback_p = (solClient_flow_eventCallbackFunc_t)flowEventCallback; - - return solClient_session_createFlow(flowPropsP, opaqueSession_p, opaqueFlow_p, flowCreateFuncInfo, sizeof(*flowCreateFuncInfo)); + solClient_flow_createFuncInfo_t flowCreateFuncInfo; + flowCreateFuncInfo.rxMsgInfo.callback_p = flowMessageReceiveCallback; + flowCreateFuncInfo.rxMsgInfo.user_p = (void *)flowID; + flowCreateFuncInfo.eventInfo.callback_p = (solClient_flow_eventCallbackFunc_t)flowEventCallback; + flowCreateFuncInfo.eventInfo.user_p = (void *)flowID; + // allocate these struct fields too + flowCreateFuncInfo.rxInfo.user_p = NULL; + flowCreateFuncInfo.rxInfo.callback_p = NULL; + + return solClient_session_createFlow(flowPropsP, opaqueSession_p, opaqueFlow_p, &flowCreateFuncInfo, sizeof(flowCreateFuncInfo)); } solClient_returnCode_t diff --git a/internal/ccsmp/ccsmp_helper.h b/internal/ccsmp/ccsmp_helper.h index 356d94c..c4cdcc1 100644 --- a/internal/ccsmp/ccsmp_helper.h +++ b/internal/ccsmp/ccsmp_helper.h @@ -50,7 +50,6 @@ solClient_returnCode_t SessionFlowCreate( solClient_opaqueSession_pt opaqueSession_p, solClient_propertyArray_pt flowPropsP, solClient_opaqueFlow_pt *opaqueFlow_p, - solClient_flow_createFuncInfo_t *flowCreateFuncInfo, solClient_uint64_t flowID_p); solClient_returnCode_t FlowTopicSubscribeWithDispatch( From 2153c35e55e45c642d59279a3d06673aecff3932 Mon Sep 17 00:00:00 2001 From: Chris Morgan <17009318+cjwmorgan-sol@users.noreply.github.com> Date: Mon, 22 Apr 2024 10:35:30 -0400 Subject: [PATCH 6/8] SOL-117089: updated git actions checks for go version 1.22, go vet and staticcheck Signed-off-by: Chris Morgan <17009318+cjwmorgan-sol@users.noreply.github.com> --- .github/workflows/test.yml | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 78dcd78..fac46a1 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -42,7 +42,7 @@ jobs: - name: Setup Go environment uses: actions/setup-go@v2.1.3 with: - go-version: '1.21' + go-version: '1.22' check-latest: true - name: Check Go Version run: go version @@ -66,6 +66,28 @@ jobs: echo "$OUTPUT" exit 1 fi + + - name: Runs go vet + if: ${{ success() }} + run: | + OUTPUT=$(go vet ./... 2>&1) + if [ ! -z "$OUTPUT" ]; then + echo "go vet failed on the following:" + echo "$OUTPUT" + exit 1 + fi + + - name: Runs staticcheck + if: ${{ success() }} + run: | + go install honnef.co/go/tools/cmd/staticcheck@v0.3.3 + OUTPUT=$(staticcheck --checks=all ./...) + if [ ! -z "$OUTPUT" ]; then + echo "staticcheck failed on the following:" + echo "$OUTPUT" + exit 1 + fi + - name: Runs unit tests if: ${{ success() }} run: go test -coverprofile ./unitcoverage.out ./... From e525fca03e5591c829fcbe91f3fdd2e5b8e743ee Mon Sep 17 00:00:00 2001 From: Chris Morgan <17009318+cjwmorgan-sol@users.noreply.github.com> Date: Mon, 22 Apr 2024 10:44:52 -0400 Subject: [PATCH 7/8] SOL-117089: bump static check version from 0.3.3 to 0.4.7 for go version 1.22 Signed-off-by: Chris Morgan <17009318+cjwmorgan-sol@users.noreply.github.com> --- .github/workflows/test.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index fac46a1..e7b0224 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -80,7 +80,8 @@ jobs: - name: Runs staticcheck if: ${{ success() }} run: | - go install honnef.co/go/tools/cmd/staticcheck@v0.3.3 + # use pinned version of the tool but check if this needs an update on go version bump + go install honnef.co/go/tools/cmd/staticcheck@v0.4.7 OUTPUT=$(staticcheck --checks=all ./...) if [ ! -z "$OUTPUT" ]; then echo "staticcheck failed on the following:" From 5abadf2c7df4c2a1c942743a7818b7ae957b7492 Mon Sep 17 00:00:00 2001 From: Oseme Odigie Date: Fri, 3 May 2024 11:16:57 -0400 Subject: [PATCH 8/8] feat: marked flaky tests and skipped them due to SOL-117804 --- test/messaging_service_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/messaging_service_test.go b/test/messaging_service_test.go index edee2ee..a26c931 100644 --- a/test/messaging_service_test.go +++ b/test/messaging_service_test.go @@ -342,9 +342,12 @@ var _ = Describe("MessagingService Lifecycle", func() { helpers.TestConnectDisconnectMessagingService(builder) }) - Context("with a bad server certificate installed on the broker", func() { + // Skip these tests; they are currently failing in Git actions see SOL-117804 + Context("with a bad server certificate installed on the broker", Label("flaky-tests"), func() { var invalidServerCertificate string JustBeforeEach(func() { + Skip("Currently failing in Git actions - SOL-117804") + certContent, err := ioutil.ReadFile(invalidServerCertificate) Expect(err).ToNot(HaveOccurred()) // Git actions seems to have some trouble with this particular SEMP request and occasionally gets EOF errors