From 7ce7d3d3f2f11061794dd99759e10608c09b9f2c Mon Sep 17 00:00:00 2001 From: Nathan Glenn Date: Thu, 31 Aug 2023 17:03:23 -0500 Subject: [PATCH] Roll back 9.6.1 RhsHandler and ClientMessageHandler change Ultimately it's not possible to simply use C-types in SML and get a portable lib out of it, since C++ classes are used extensively. Creating a C-wrapper for libSoar would be a large project of its own, and using the C++ code as-is is not really viable at this point; we would need a SWIG-like tool (why on earth does SWIG not support C++ to C wrapping?). Fix and remove outstanding TODOs from the whole refactoring. Leave the new APIs in place, as they are more ergonomic C++, anyway. Remove all logic related to the buffer resizing, and remove duplicate code where possible. Add documentation to the new methods. --- Core/ClientSML/src/sml_ClientEvents.h | 39 ++--------- Core/ClientSML/src/sml_ClientKernel.cpp | 55 +++++++--------- Core/ClientSML/src/sml_ClientKernel.h | 19 ++++-- .../CSharp/CSharpCallbackByHand.h | 14 +--- Core/ClientSMLSWIG/Java/JavaCallbackByHand.h | 17 +---- .../Python/Python_sml_ClientInterface.i | 64 ++----------------- Core/ClientSMLSWIG/README.md | 4 ++ .../Tcl/Tcl_sml_ClientInterface.i | 14 +--- Tcl/src/TclSoarLib.cpp | 12 +--- Tcl/src/TclSoarLib.h | 2 +- UnitTests/SoarUnitTests/FullTests.cpp | 3 +- UnitTests/SoarUnitTests/SimpleListener.cpp | 10 ++- UnitTests/SoarUnitTests/SimpleListener.hpp | 12 ++-- UnitTests/SoarUnitTests/handlers.cpp | 49 ++++---------- UnitTests/SoarUnitTests/handlers.hpp | 10 +-- 15 files changed, 91 insertions(+), 233 deletions(-) diff --git a/Core/ClientSML/src/sml_ClientEvents.h b/Core/ClientSML/src/sml_ClientEvents.h index e2d32e5eeb..12add8aef8 100644 --- a/Core/ClientSML/src/sml_ClientEvents.h +++ b/Core/ClientSML/src/sml_ClientEvents.h @@ -69,50 +69,21 @@ namespace sml // You need to delete ClientXML objects you create and you should not delete the pXML object you are passed. typedef void (*XMLEventHandler)(smlXMLEventId id, void* pUserData, Agent* pAgent, ClientXML* pXML) ; - -// TODO: move to Kernel::AddRhsHandler, which clients actually see. Expand on why this is the way it is. -// Maintainer note: RhsEventHandler and ClientMessageHandler require special handling: -// RETURN: returns pointer to buff. If buff is not large enough, it will return NULL and set buffSize to a new size. Reallocate the new size and call again. -// Implementations should save a static value locally to be returned when the client retries the call. This also means that the client -// MUST re-do the call if NULL was returned, or else the next call will return an unrelated value. -// Recommended implementation: -// // at beginning of function: -// static std::string prevResult; -// if ( !prevResult.empty() ) -// { -// strncpy( buf, prevResult.c_str(), *bufSize ); -// prevResult = ""; -// return buf; -// } -// ... -// // at end of function -// if ( resultString.length() + 1 > *bufSize ) -// { -// *bufSize = resultString.length() + 1; -// prevResult = resultString; -// return NULL; -// } -// strcpy( buf, resultString.c_str() ); -// return buf; - // Handler for RHS (right hand side) function firings // pFunctionName and pArgument define the RHS function being called (the client may parse pArgument to extract other values) // The return value is a string which allows the RHS function to create a symbol: e.g. ^att (exec plus 2 2) producing ^att 4 -// SEE MAINTAINER NOTE ABOVE! - typedef char const *(*RhsEventHandler)(smlRhsEventId id, void* pUserData, Agent* pAgent, - char const* pFunctionName, char const* pArgument, int *buffSize, char *buff) ; + typedef std::string (*RhsEventHandler)(smlRhsEventId id, void* pUserData, Agent* pAgent, + char const* pFunctionName, char const* pArgument) ; using RhsEventHandlerCpp = std::function; // Handler for a generic "client message". The content is determined by the client sending this data. // The message is sent as a simple string and the response is also a string. The string can contain data that is intended to be parsed, // such as a simple series of integers up to a complete XML message. - // SEE MAINTAINER NOTE ABOVE! - // TODO: add alternative similar to RhsEventHandlerCpp above - typedef char const *(*ClientMessageHandler)(smlRhsEventId id, void *pUserData, Agent *pAgent, - char const *pFunctionName, char const *pArgument, int *buffSize, char *buff); + typedef std::string(*ClientMessageHandler)(smlRhsEventId id, void *pUserData, Agent *pAgent, + char const *pClientName, char const *pArgument); - using ClientMessageHandlerCpp = std::function; + using ClientMessageHandlerCpp = std::function; // We'll store a handler function together with a generic pointer to data of the user's choosing // (which is then passed back into the handler when the event occurs). diff --git a/Core/ClientSML/src/sml_ClientKernel.cpp b/Core/ClientSML/src/sml_ClientKernel.cpp index 7e905b903e..c5984b84fb 100644 --- a/Core/ClientSML/src/sml_ClientKernel.cpp +++ b/Core/ClientSML/src/sml_ClientKernel.cpp @@ -671,8 +671,6 @@ void Kernel::ReceivedRhsEvent(smlRhsEventId id, AnalyzeXML* pIncoming, ElementXM return ; } - // TODO: since we're only using the name, we could get rid of the handler+data wrapper class - // and iterate over name/handler key/vals here instead of looking up the name in the wrapper // Look up the handler(s) from the map RhsEventMap::ValueList* pHandlers = m_RhsEventMap.getList(pFunctionName) ; @@ -2078,7 +2076,7 @@ int Kernel::InternalAddRhsFunction(smlRhsEventId id, char const* pRhsFunctionNam bool found = m_RhsEventMap.findFirstValueByTest(&test, &optionalFoundHandler) ; if (found && optionalFoundHandler.m_Handler != 0) { - // TODO: log a warning here to help catch copy/paste errors in the client + std::cerr << "WARNING: Attempt to register a duplicate RHS function handler for " << pRhsFunctionName << std::endl ; return optionalFoundHandler.getCallbackID() ; } @@ -2134,37 +2132,21 @@ bool Kernel::InternalRemoveRhsFunction(smlRhsEventId id, int callbackID) return true ; } -// TODO: document -RhsEventHandlerCpp c2cppHandler(RhsEventHandler handler, void* pUserData) { +// RHS function and client message handlers can be added via 2 separate API's; this helper converts +// the function pointer-style handlers into std::function-style handlers, because the +// std::function-style is used internally. +RhsEventHandlerCpp funPointer2StdFunction(RhsEventHandler handler, void* pUserData) { return [handler, pUserData]( smlRhsEventId id, Agent* pAgent, char const* pFunctionName, char const* pArgument) -> std::string { - char buffer[SML_INITIAL_BUFFER_SIZE] = {0}; - char *buff = buffer; - std::string result; - int buffSize = sizeof(buffer); - - const char *retVal = handler(id, pUserData, pAgent, pFunctionName, pArgument, &buffSize, buff) ; - - if ( !retVal ) - { - buff = new char[buffSize]; - - result = handler(id, pUserData, pAgent, pFunctionName, pArgument, &buffSize, buff) ; - // TODO: check for NIL again and print error of some kind - delete [] buff; - } - else { - result = retVal; - }; - return result; + return handler(id, pUserData, pAgent, pFunctionName, pArgument) ; }; } /************************************************************* -* @brief Register a handler for a RHS (right hand side) function. +* @brief Register a handler for an RHS (right hand side) function. * This function can be called in the RHS of a production firing * allowing a user to quickly extend Soar with custom methods added to the client. * @@ -2189,10 +2171,18 @@ RhsEventHandlerCpp c2cppHandler(RhsEventHandler handler, void* pUserData) { *************************************************************/ int Kernel::AddRhsFunction(char const* pRhsFunctionName, RhsEventHandler handler, void* pUserData, bool addToBack) { - RhsEventHandlerCpp newHandler = c2cppHandler(handler, pUserData); + RhsEventHandlerCpp newHandler = funPointer2StdFunction(handler, pUserData); return AddRhsFunction(pRhsFunctionName, newHandler, addToBack) ; } + +/***************************************************** + * @brief Register a handler for an RHS (right hand side) function. This is functionally the + * same as the method with the same name that takes a function pointer, but it uses + * std::function instead, which is more ergonomic C++. + * + * @see sml::Kernel::AddRhsFunction(char const* pClientName, RhsEventHandler handler, void* pUserData, bool addToBack) +*/ int Kernel::AddRhsFunction(char const* pRhsFunctionName, RhsEventHandlerCpp handler, bool addToBack) { smlRhsEventId id = smlEVENT_RHS_USER_FUNCTION ; @@ -2217,9 +2207,6 @@ bool Kernel::RemoveRhsFunction(int callbackID) * When the original client sends a message, the RHS function handler is called to process and (optionally) return * a message to the caller. * -* Multiple handlers can be registered for a given message type and the results will be concatenated together and returned -* to the original caller. (This is expected to be an usual situation). -* * A RHS (right hand side) function handler is used just to reduce the number of types in the system and because it is sufficient * for this purpose. * @@ -2236,11 +2223,17 @@ bool Kernel::RemoveRhsFunction(int callbackID) *************************************************************/ int Kernel::RegisterForClientMessageEvent(char const* pClientName, ClientMessageHandler handler, void* pUserData, bool addToBack) { - RhsEventHandlerCpp newHandler = c2cppHandler(handler, pUserData); + RhsEventHandlerCpp newHandler = funPointer2StdFunction(handler, pUserData); return RegisterForClientMessageEvent(pClientName, newHandler, addToBack) ; } -// TODO: document +/***************************************************** + * @brief Register a handler for receiving generic messages sent from another client. This is + * functionally the same as the method with the same name that takes a function pointer, but it + * uses std::function instead, which is more ergonomic C++. + * + * @see sml::Kernel::RegisterForClientMessageEvent(char const* pClientName, ClientMessageHandler handler, void* pUserData, bool addToBack) +*/ int Kernel::RegisterForClientMessageEvent(char const* pClientName, ClientMessageHandlerCpp handler, bool addToBack) { smlRhsEventId id = smlEVENT_CLIENT_MESSAGE ; // We actually use the RHS function code internally to process this message (since it's almost exactly like calling a RHS function that's diff --git a/Core/ClientSML/src/sml_ClientKernel.h b/Core/ClientSML/src/sml_ClientKernel.h index 38f94d2363..5342f2c7cd 100644 --- a/Core/ClientSML/src/sml_ClientKernel.h +++ b/Core/ClientSML/src/sml_ClientKernel.h @@ -19,8 +19,6 @@ #include "sml_ClientEvents.h" #include "sml_ListMap.h" -#define SML_INITIAL_BUFFER_SIZE 1024 - // Forward declare so clients can use this struct ElementXML_InterfaceStruct; typedef ElementXML_InterfaceStruct* ElementXML_Handle ; @@ -781,7 +779,14 @@ namespace sml * @returns Unique ID for this callback. Required when unregistering this callback. *************************************************************/ int AddRhsFunction(char const* pRhsFunctionName, RhsEventHandler handler, void* pUserData, bool addToBack = true) ; - // TODO: document + + /***************************************************** + * @brief Register a handler for an RHS (right hand side) function. This is functionally the + * same as the method with the same name that takes a function pointer, but it uses + * std::function instead, which is more ergonomic C++. + * + * @see sml::Kernel::AddRhsFunction(char const* pClientName, RhsEventHandler handler, void* pUserData, bool addToBack) + */ int AddRhsFunction(char const* pRhsFunctionName, RhsEventHandlerCpp handler, bool addToBack = true) ; /************************************************************* @@ -818,7 +823,13 @@ namespace sml * @returns Unique ID for this callback. Required when unregistering this callback. *************************************************************/ int RegisterForClientMessageEvent(char const* pClientName, ClientMessageHandler handler, void* pUserData, bool addToBack = true) ; - // TODO: document + /***************************************************** + * @brief Register a handler for receiving generic messages sent from another client. This is + * functionally the same as the method with the same name that takes a function pointer, but it + * uses std::function instead, which is more ergonomic C++. + * + * @see sml::Kernel::RegisterForClientMessageEvent(char const* pClientName, ClientMessageHandler handler, void* pUserData, bool addToBack) + */ int RegisterForClientMessageEvent(char const* pClientName, ClientMessageHandlerCpp handler, bool addToBack = true) ; /************************************************************* diff --git a/Core/ClientSMLSWIG/CSharp/CSharpCallbackByHand.h b/Core/ClientSMLSWIG/CSharp/CSharpCallbackByHand.h index ca5931639e..75824dc572 100644 --- a/Core/ClientSMLSWIG/CSharp/CSharpCallbackByHand.h +++ b/Core/ClientSMLSWIG/CSharp/CSharpCallbackByHand.h @@ -817,7 +817,7 @@ typedef char const* (STDCALL* ClientMessageCallback)(int eventID, CallbackDataPt // This is the C++ handler which will be called by clientSML when the event fires. // Then from here we need to call back to C# to pass back the message. -static const std::string RhsEventHandler(sml::smlRhsEventId /*id*/, CallbackDataPtr pUserData, sml::Agent* pAgent, char const* pFunctionName, char const* pArgument) +static std::string RhsEventHandler(sml::smlRhsEventId /*id*/, CallbackDataPtr pUserData, sml::Agent* pAgent, char const* pFunctionName, char const* pArgument) { // The user data is the class we declared above, where we store the Java data to use in the callback. CSharpCallbackData* pData = (CSharpCallbackData*)pUserData ; @@ -834,14 +834,6 @@ static const std::string RhsEventHandler(sml::smlRhsEventId /*id*/, CallbackData return res; } -static const sml::RhsEventHandlerCpp getRhsEventHandler(void *pUserData) -{ - return [pUserData](sml::smlRhsEventId id, sml::Agent *pAgent, char const *pFunctionName, char const *pArgument) -> const std::string - { - return RhsEventHandler(id, pUserData, pAgent, pFunctionName, pArgument); - }; -} - SWIGEXPORT intptr_t SWIGSTDCALL CSharp_Kernel_AddRhsFunction(kernelPtr jarg1, char const* pFunctionName, kernelPtr jkernel, callbackFnPtr jarg3, CallbackDataPtr jdata) { // jarg1 is the C++ Kernel object @@ -856,7 +848,7 @@ SWIGEXPORT intptr_t SWIGSTDCALL CSharp_Kernel_AddRhsFunction(kernelPtr jarg1, ch CSharpCallbackData* pData = CreateCSharpCallbackDataKernel(jkernel, 0, jarg3, jdata) ; // Register our handler. When this is called we'll call back to the client method. - pData->m_CallbackID = arg1->AddRhsFunction(pFunctionName, getRhsEventHandler(pData)) ; + pData->m_CallbackID = arg1->AddRhsFunction(pFunctionName, &RhsEventHandler, pData) ; // Pass the callback info back to the client. We need to do this so we can delete this later when the method is unregistered return reinterpret_cast(pData) ; @@ -899,7 +891,7 @@ SWIGEXPORT intptr_t SWIGSTDCALL CSharp_Kernel_RegisterForClientMessageEvent(kern CSharpCallbackData* pData = CreateCSharpCallbackDataKernel(jkernel, 0, jarg3, jdata) ; // Register our handler. When this is called we'll call back to the client method. - pData->m_CallbackID = arg1->RegisterForClientMessageEvent(pClientName, getRhsEventHandler(pData)) ; + pData->m_CallbackID = arg1->RegisterForClientMessageEvent(pClientName, &RhsEventHandler, pData) ; // Pass the callback info back to the client. We need to do this so we can delete this later when the method is unregistered return reinterpret_cast(pData) ; diff --git a/Core/ClientSMLSWIG/Java/JavaCallbackByHand.h b/Core/ClientSMLSWIG/Java/JavaCallbackByHand.h index d565ded0c5..072398307e 100644 --- a/Core/ClientSMLSWIG/Java/JavaCallbackByHand.h +++ b/Core/ClientSMLSWIG/Java/JavaCallbackByHand.h @@ -234,7 +234,7 @@ static std::string StringEventHandler(sml::smlStringEventId id, void* pUserData, } -static const std::string handleRhsEvent(sml::smlRhsEventId id, void *pUserData, sml::Agent *pAgent, +static std::string handleRhsEvent(sml::smlRhsEventId id, void *pUserData, sml::Agent *pAgent, char const *pFunctionName, char const *pArgument) { // The user data is the class we declared above, where we store the Java data to use in the callback. @@ -273,17 +273,6 @@ static const std::string handleRhsEvent(sml::smlRhsEventId id, void *pUserData, return resultStr; } -// This is the C++ handler which will be called by clientSML when the event fires. -// Then from here we need to call back to Java to pass back the message. -static const sml::RhsEventHandlerCpp getRhsEventHandler(void *pUserData) -{ - return [pUserData](sml::smlRhsEventId id, sml::Agent *pAgent, char const *pFunctionName, char const *pArgument) - { - return handleRhsEvent(id, pUserData, pAgent, pFunctionName, pArgument); - }; -} - - #ifdef __cplusplus // We expose the public methods in a DLL with C naming (not C++ mangled names) extern "C" { @@ -933,7 +922,7 @@ JNIEXPORT jlong JNICALL Java_sml_smlJNI_Kernel_1AddRhsFunction(JNIEnv *jenv, jcl JavaCallbackData* pJavaData = CreateJavaCallbackData(false, jenv, jcls, jarg1, 0, jarg3, jarg4, "rhsFunctionHandler", "(ILjava/lang/Object;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;)Ljava/lang/String;", jarg6) ; // Register our handler. When this is called we'll call back to the Java method. - pJavaData->m_CallbackID = arg1->AddRhsFunction(pFunctionName, getRhsEventHandler(pJavaData)) ; + pJavaData->m_CallbackID = arg1->AddRhsFunction(pFunctionName, &handleRhsEvent, pJavaData) ; // Release the string we got from Java jenv->ReleaseStringUTFChars(jarg2, pFunctionName); @@ -976,7 +965,7 @@ JNIEXPORT jlong JNICALL Java_sml_smlJNI_Kernel_1RegisterForClientMessageEvent(JN JavaCallbackData* pJavaData = CreateJavaCallbackData(false, jenv, jcls, jarg1, 0, jarg3, jarg4, "clientMessageHandler", "(ILjava/lang/Object;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;)Ljava/lang/String;", jarg6) ; // Register our handler. When this is called we'll call back to the Java method. - pJavaData->m_CallbackID = arg1->RegisterForClientMessageEvent(pFunctionName, getRhsEventHandler(pJavaData)) ; + pJavaData->m_CallbackID = arg1->RegisterForClientMessageEvent(pFunctionName, &handleRhsEvent, pJavaData) ; // Release the string we got from Java jenv->ReleaseStringUTFChars(jarg2, pFunctionName); diff --git a/Core/ClientSMLSWIG/Python/Python_sml_ClientInterface.i b/Core/ClientSMLSWIG/Python/Python_sml_ClientInterface.i index 8f5fd9a1d6..b6b7143308 100644 --- a/Core/ClientSMLSWIG/Python/Python_sml_ClientInterface.i +++ b/Core/ClientSMLSWIG/Python/Python_sml_ClientInterface.i @@ -282,7 +282,7 @@ PyGILState_Release(gstate); /* Release the thread. No Python API allowed beyond this point. */ } - const std::string PythonRhsEventCallback(sml::smlRhsEventId id, void* pUserData, sml::Agent* pAgent, char const* pFunctionName, char const* pArgument) + static std::string PythonRhsEventCallback(sml::smlRhsEventId id, void* pUserData, sml::Agent* pAgent, char const* pFunctionName, char const* pArgument) { PyGILState_STATE gstate; gstate = PyGILState_Ensure(); /* Get the thread. No Python API allowed before this point. */ @@ -299,6 +299,7 @@ if(!result) { show_exception_and_exit("RHS event", id); } else if (!PyUnicode_Check(result)) { + // TODO: perhaps log a warning here? return ""; } @@ -310,63 +311,6 @@ return res; } - const sml::RhsEventHandlerCpp getPythonRhsEventCallback(void *pUserData) - { - return [pUserData](sml::smlRhsEventId id, sml::Agent *pAgent, char const *pFunctionName, char const *pArgument) -> const std::string - { - return PythonRhsEventCallback(id, pUserData, pAgent, pFunctionName, pArgument); - }; - } - - const char *PythonClientMessageEventCallback(sml::smlRhsEventId id, void* pUserData, sml::Agent* pAgent, char const* pClientName, char const* pMessage, int *bufSize, char *buf) - { - // Previous result was cached, meaning client should be calling again to get it - // return that result and clear the cache - static std::string prevResult; - if ( !prevResult.empty() ) - { - strncpy( buf, prevResult.c_str(), *bufSize ); - - prevResult = ""; - - return buf; - } - - PyGILState_STATE gstate; - gstate = PyGILState_Ensure(); /* Get the thread. No Python API allowed before this point. */ - - PythonUserData* pud = static_cast(pUserData); - - PyObject* agent = SWIG_NewInstanceObj((void *) pAgent, SWIGTYPE_p_sml__Agent,0); - PyObject* args = Py_BuildValue("(iOOss)", id, pud->userdata, agent, pClientName, pMessage); - PyObject* result = PyObject_Call(pud->func, args, NULL); - - Py_DECREF(agent); - Py_DECREF(args); - if(!result) { - show_exception_and_exit("client message event", id); - } else if (!PyUnicode_Check(result)) { - return ""; - } - - std::string res = PyUnicode_AsUTF8 (result); - Py_DECREF(result); - - PyGILState_Release(gstate); /* Release the thread. No Python API allowed beyond this point. */ - - // Too long to fit in the buffer; cache result and signal client with - // NULL return value to call again with a larger buffer - if ( res.length() + 1 > *bufSize ) - { - *bufSize = res.length() + 1; - prevResult = res; - return NULL; - } - strcpy( buf, res.c_str() ); - - return buf; - } - PythonUserData* CreatePythonUserData(PyObject* func, PyObject* userData) { PythonUserData* pud = new PythonUserData(); PyGILState_STATE gstate; @@ -532,13 +476,13 @@ long AddRhsFunction(char const* pRhsFunctionName, PyObject* func, PyObject* userData, bool addToBack = true) { PythonUserData* pud = CreatePythonUserData(func, userData); - pud->callbackid = self->AddRhsFunction(pRhsFunctionName, getPythonRhsEventCallback((void*)pud), addToBack); + pud->callbackid = self->AddRhsFunction(pRhsFunctionName, &PythonRhsEventCallback, (void*)pud, addToBack); return (long)pud; } long RegisterForClientMessageEvent(char const* pClientName, PyObject* pMessageHandler, PyObject* userData, bool addToBack = true) { PythonUserData* pud = CreatePythonUserData(pMessageHandler, userData); - pud->callbackid = self->RegisterForClientMessageEvent(pClientName, PythonClientMessageEventCallback, (void*)pud, addToBack); + pud->callbackid = self->RegisterForClientMessageEvent(pClientName, &PythonRhsEventCallback, (void*)pud, addToBack); return (long)pud; } diff --git a/Core/ClientSMLSWIG/README.md b/Core/ClientSMLSWIG/README.md index 98e9befdf2..53da5e7493 100644 --- a/Core/ClientSMLSWIG/README.md +++ b/Core/ClientSMLSWIG/README.md @@ -7,6 +7,10 @@ * `*.i`` files contain directives for SWIG. `sml_ClientInterface.i` in the top-level directory contains directives common to each target language. * `*CallbackByHand.h` files contain manually-written C wrappers related to callback functions. These could not be generated automatically by SWIG (TODO: why?). +## New SML Members + +If new functions or classes are added to the `sml` namespace, SWIG will try to export them automatically. If you want to prevent this, you need to add a `%ignore` directive to `sml_ClientInterface.i`. You will also need to do this for any functions that require a custom implementation for a target language. + ## Portability SWIG generates pure C wrappers for the SML client library, which are then compiled into shared libraries for each target language. SWIG also generates idiomatic code in each target language for loading and using the library. Because the library is compiled from C, it is much more portable across platforms, and the symbol names are not mangled. diff --git a/Core/ClientSMLSWIG/Tcl/Tcl_sml_ClientInterface.i b/Core/ClientSMLSWIG/Tcl/Tcl_sml_ClientInterface.i index fa46bc6abc..619300bea3 100644 --- a/Core/ClientSMLSWIG/Tcl/Tcl_sml_ClientInterface.i +++ b/Core/ClientSMLSWIG/Tcl/Tcl_sml_ClientInterface.i @@ -162,7 +162,7 @@ } // RHS function used to exec Tcl code from Soar - const std::string TclRhsEventCallback(sml::smlRhsEventId, void* pUserData, sml::Agent* pAgent, char const* pFunctionName, + static std::string TclRhsEventCallback(sml::smlRhsEventId, void* pUserData, sml::Agent* pAgent, char const* pFunctionName, char const* pArgument) { TclUserData* tud = static_cast(pUserData); @@ -189,14 +189,6 @@ return sres; } - const sml::RhsEventHandlerCpp getTclRhsEventCallback(void *pUserData) - { - return [pUserData](sml::smlRhsEventId id, sml::Agent *pAgent, char const *pFunctionName, char const *pArgument) -> const std::string - { - return TclRhsEventCallback(id, pUserData, pAgent, pFunctionName, pArgument); - }; - } - void TclAgentEventCallback(sml::smlAgentEventId id, void* pUserData, sml::Agent* agent) { // we can ignore the id parameter because it's already in the script (from when we registered it) @@ -589,13 +581,13 @@ // to have different names in Tcl (and they are both strings -- in C++ they're not the same type; hence the separation there) intptr_t AddRhsFunction(Tcl_Interp* interp, char const* pRhsFunctionName, char* userData, bool addToBack = true) { TclUserData* tud = CreateTclUserData(sml::smlEVENT_RHS_USER_FUNCTION, pRhsFunctionName, userData, interp); - tud->callbackid = self->AddRhsFunction(pRhsFunctionName, getTclRhsEventCallback((void*)tud), addToBack); + tud->callbackid = self->AddRhsFunction(pRhsFunctionName, &TclRhsEventCallback, (void*)tud, addToBack); return (intptr_t)tud; } intptr_t RegisterForClientMessageEvent(Tcl_Interp* interp, char const* pClientName, char const* pMessageHandler, char* userData, bool addToBack = true) { TclUserData* tud = CreateTclUserData(sml::smlEVENT_RHS_USER_FUNCTION, pMessageHandler, userData, interp); - tud->callbackid = self->RegisterForClientMessageEvent(pClientName, getTclRhsEventCallback((void*)tud), addToBack); + tud->callbackid = self->RegisterForClientMessageEvent(pClientName, &TclRhsEventCallback, (void*)tud, addToBack); return (intptr_t)tud; } diff --git a/Tcl/src/TclSoarLib.cpp b/Tcl/src/TclSoarLib.cpp index 5df3af80a4..f420411678 100644 --- a/Tcl/src/TclSoarLib.cpp +++ b/Tcl/src/TclSoarLib.cpp @@ -161,7 +161,7 @@ TclSoarLib::~TclSoarLib() //cout << "TclSoarLib destroyed" << std::endl; } -const char *TclSoarLib::tclRHS( sml::smlRhsEventId, void *pData, sml::Agent *pAgent, char const *pFunc, char const *pArg, int *buffSize, char *buff ) +std::string TclSoarLib::tclRHS( sml::smlRhsEventId, void *pData, sml::Agent *pAgent, char const *pFunc, char const *pArg) { TclSoarLib *pLib = (TclSoarLib *)pData; @@ -174,15 +174,7 @@ const char *TclSoarLib::tclRHS( sml::smlRhsEventId, void *pData, sml::Agent *pAg comm += "}"; pLib->GlobalEval( comm, res); - - if ( res.size() + 1 > *buffSize ) - { - *buffSize = res.size() + 1; - return NULL; - } - strcpy( buff, res.c_str() ); - - return buff; + return res; } void TclSoarLib::send_thread_command(int type, std::string info){ diff --git a/Tcl/src/TclSoarLib.h b/Tcl/src/TclSoarLib.h index 55eb24c6ca..b60195a22b 100644 --- a/Tcl/src/TclSoarLib.h +++ b/Tcl/src/TclSoarLib.h @@ -97,7 +97,7 @@ class EXPORT TclSoarLib bool initialize_Master(); bool initialize_Tcl_Interpreter(); - static const char *tclRHS( sml::smlRhsEventId id, void *pData, sml::Agent *pAgent, char const *pFunc, char const *pArg, int *bufSize, char *buf ); + static std::string tclRHS( sml::smlRhsEventId id, void *pData, sml::Agent *pAgent, char const *pFunc, char const *pArg); // Methods to send commands to the tcl interpreter bool evaluateDirCommand(const std::string command); diff --git a/UnitTests/SoarUnitTests/FullTests.cpp b/UnitTests/SoarUnitTests/FullTests.cpp index 50d53c651a..ed468ac664 100644 --- a/UnitTests/SoarUnitTests/FullTests.cpp +++ b/UnitTests/SoarUnitTests/FullTests.cpp @@ -422,8 +422,7 @@ void FullTests_Parent::testClientMessageHandler() int clientCallbackCpp = m_pKernel->RegisterForClientMessageEvent("test-client-cpp", Handlers::GetClientMessageHandlerCpp(&clientHandlerReceivedCpp)) ; // This is a bit dopey--but we'll send a message to ourselves for this test - // use a large message to exercise buffer re-allocation logic - std::string message(SML_INITIAL_BUFFER_SIZE + 100, 'x'); + std::string message("foo-bar-baz-qux"); std::string response = m_pKernel->SendClientMessage(agent, "test-client", message.c_str()); no_agent_assertTrue(clientHandlerReceived); std::string expected = "handler-message" + message; diff --git a/UnitTests/SoarUnitTests/SimpleListener.cpp b/UnitTests/SoarUnitTests/SimpleListener.cpp index dbeb26adf4..3cd7dc9cd5 100644 --- a/UnitTests/SoarUnitTests/SimpleListener.cpp +++ b/UnitTests/SoarUnitTests/SimpleListener.cpp @@ -25,18 +25,16 @@ std::string listenerRhsFunctionHandler(sml::smlRhsEventId id, void* pUserData, s } bool SimpleListener::shutdownMessageReceived = false; -const char *SimpleListener::MyClientMessageHandler(sml::smlRhsEventId, void* pUserData, sml::Agent*, char const*, char const* pMessage, int *bufSize, char *buf) +std::string SimpleListener::MyClientMessageHandler(sml::smlRhsEventId, void* pUserData, sml::Agent*, char const*, char const* pMessage) { if (pMessage && std::string(pMessage) == "shutdown") { //std::cout << "SimpleListener got shutdown message." << std::endl; shutdownMessageReceived = true; - strcpy( buf, "ok" ); + return "ok"; } - else - strcpy( buf, "unknown command" ); - - return buf; + else + return "unknown command"; } // Create a process that listens for remote commands and lives diff --git a/UnitTests/SoarUnitTests/SimpleListener.hpp b/UnitTests/SoarUnitTests/SimpleListener.hpp index 6f33cffe61..b0ad66a4b4 100644 --- a/UnitTests/SoarUnitTests/SimpleListener.hpp +++ b/UnitTests/SoarUnitTests/SimpleListener.hpp @@ -9,18 +9,18 @@ class SimpleListener { public: SimpleListener(int life); - + int run(); // returns zero on success, nonzero failure - - static const char *MyClientMessageHandler(sml::smlRhsEventId id, void* pUserData, sml::Agent* pAgent, char const* pMessageType, char const* pMessage, int *bufSize, char *buf); - + + static std::string MyClientMessageHandler(sml::smlRhsEventId id, void* pUserData, sml::Agent* pAgent, char const* pMessageType, char const* pMessage); + private: int life; - + // Choose how to connect (usually use NewThread) but for // testing currentThread can be helpful. bool useCurrentThread; - + static bool shutdownMessageReceived; }; diff --git a/UnitTests/SoarUnitTests/handlers.cpp b/UnitTests/SoarUnitTests/handlers.cpp index 733d796e0e..7a3b1705e2 100644 --- a/UnitTests/SoarUnitTests/handlers.cpp +++ b/UnitTests/SoarUnitTests/handlers.cpp @@ -70,23 +70,15 @@ void Handlers::MyProductionHandler(sml::smlProductionEventId id, void* pUserData no_agent_assertTrue(id == sml::smlEVENT_BEFORE_PRODUCTION_REMOVED); } -const char *Handlers::MyClientMessageHandler(sml::smlRhsEventId, void* pUserData, sml::Agent*, char const*, char const* pMessage, int *bufSize, char *buf) +std::string Handlers::MyClientMessageHandler(sml::smlRhsEventId, void* pUserData, sml::Agent*, char const*, char const* pMessage) { std::stringstream res; res << "handler-message" << pMessage; - if ( res.str().size() + 1 > *bufSize ) - { - *bufSize = res.str().size() + 1; - return NULL; - } - strcpy( buf, res.str().c_str() ); - no_agent_assertTrue(pUserData); bool* pHandlerReceived = static_cast< bool* >(pUserData); *pHandlerReceived = true; - - return buf; + return res.str(); } const sml::ClientMessageHandlerCpp Handlers::GetClientMessageHandlerCpp(bool *receivedFlag) @@ -105,7 +97,7 @@ const sml::ClientMessageHandlerCpp Handlers::GetClientMessageHandlerCpp(bool *re } // This is a very dumb filter--it adds "--depth 2" to all commands passed to it. -const char *Handlers::MyFilterHandler(sml::smlRhsEventId, void* pUserData, sml::Agent*, char const*, char const* pCommandLine, int *bufSize, char *buff) +std::string Handlers::MyFilterHandler(sml::smlRhsEventId, void* pUserData, sml::Agent*, char const*, char const* pCommandLine) { soarxml::ElementXML* pXML = soarxml::ElementXML::ParseXMLFromString(pCommandLine) ; no_agent_assertTrue(pXML); @@ -123,20 +115,13 @@ const char *Handlers::MyFilterHandler(sml::smlRhsEventId, void* pUserData, sml:: std::string res(pXMLString); pXML->DeleteString(pXMLString); - delete pXML ; - - if ( res.size() + 1 > *bufSize ) - { - *bufSize = res.size() + 1; - return NULL; - } - strcpy( buff, res.c_str() ); + delete pXML; no_agent_assertTrue(pUserData); bool* pHandlerReceived = static_cast< bool* >(pUserData); *pHandlerReceived = true; - return buff; + return res; } void Handlers::MyRunEventHandler(sml::smlRunEventId, void* pUserData, sml::Agent*, sml::smlPhase) @@ -270,7 +255,7 @@ void Handlers::MyInterruptHandler(sml::smlRunEventId, void* pUserData, sml::Agen *pHandlerReceived = true; } -const char *Handlers::MyRhsFunctionHandler(sml::smlRhsEventId, void* pUserData, sml::Agent*, char const*, char const* pArgument, int *buffSize, char *buff ) +std::string Handlers::MyRhsFunctionHandler(sml::smlRhsEventId, void* pUserData, sml::Agent*, char const*, char const* pArgument) { // This is optional because the callback needs to be around for the program to function properly // we only test for this specifically in one part of clientsmltest @@ -282,15 +267,7 @@ const char *Handlers::MyRhsFunctionHandler(sml::smlRhsEventId, void* pUserData, std::stringstream res; res << "my rhs result " << pArgument; - - if ( res.str().size() + 1 > *buffSize ) - { - *buffSize = res.str().size() + 1; - return NULL; - } - strcpy( buff, res.str().c_str() ); - - return buff; + return res.str(); } @@ -463,22 +440,18 @@ void Handlers::MyOrderingRunHandler(sml::smlRunEventId /*id*/, void* pUserData, ++(*pInt); } -const char *Handlers::MyRhsFunctionFailureHandler(sml::smlRhsEventId, void*, sml::Agent*, char const*, char const* pArgument, int *buffSize, char *buff) +std::string Handlers::MyRhsFunctionFailureHandler(sml::smlRhsEventId, void*, sml::Agent*, char const*, char const* pArgument) { std::ostringstream message; message << "test-failure handler called for: " << pArgument; no_agent_assertTrue_msg(message.str().c_str(), false); - - buff[0] = 0; - return buff; + return message.str(); } -const char *Handlers::MySuccessHandler(sml::smlRhsEventId id, void* pUserData, sml::Agent* pAgent, char const* pFunctionName, char const* pArgument, int *buffSize, char *buff) +std::string Handlers::MySuccessHandler(sml::smlRhsEventId id, void* pUserData, sml::Agent* pAgent, char const* pFunctionName, char const* pArgument) { no_agent_assertTrue(pUserData); bool* pHandlerReceived = static_cast< bool* >(pUserData); *pHandlerReceived = true; - - buff[0] = 0; - return buff; + return ""; } diff --git a/UnitTests/SoarUnitTests/handlers.hpp b/UnitTests/SoarUnitTests/handlers.hpp index bc107ea07e..2da7f1c601 100644 --- a/UnitTests/SoarUnitTests/handlers.hpp +++ b/UnitTests/SoarUnitTests/handlers.hpp @@ -25,9 +25,9 @@ class Handlers static void MySystemEventHandler(sml::smlSystemEventId id, void* pUserData, sml::Kernel* pKernel); static void MyCreationHandler(sml::smlAgentEventId id, void* pUserData, sml::Agent* pAgent); static void MyProductionHandler(sml::smlProductionEventId id, void* pUserData, sml::Agent* pAgent, char const* pProdName, char const* pInstantiation); - static const char *MyClientMessageHandler(sml::smlRhsEventId id, void* pUserData, sml::Agent* pAgent, char const* pMessageType, char const* pMessage, int *buffSize, char *buff); + static std::string MyClientMessageHandler(sml::smlRhsEventId id, void* pUserData, sml::Agent* pAgent, char const* pMessageType, char const* pMessage); static const sml::ClientMessageHandlerCpp GetClientMessageHandlerCpp(bool* receivedFlag); - static const char *MyFilterHandler(sml::smlRhsEventId id, void* pUserData, sml::Agent* pAgent, char const* pMessageType, char const* pCommandLine, int *buffSize, char *buff); + static std::string MyFilterHandler(sml::smlRhsEventId id, void* pUserData, sml::Agent* pAgent, char const* pMessageType, char const* pCommandLine); static void MyRunEventHandler(sml::smlRunEventId id, void* pUserData, sml::Agent* pAgent, sml::smlPhase phase); static void MyUpdateEventHandler(sml::smlUpdateEventId id, void* pUserData, sml::Kernel* pKernel, sml::smlRunFlags runFlags); static void MyOutputNotificationHandler(void* pUserData, sml::Agent* pAgent); @@ -38,7 +38,7 @@ class Handlers static void MyPrintEventHandler(sml::smlPrintEventId id, void* pUserData, sml::Agent* pAgent, char const* pMessage); static void MyXMLEventHandler(sml::smlXMLEventId id, void* pUserData, sml::Agent* pAgent, sml::ClientXML* pXML); static void MyInterruptHandler(sml::smlRunEventId id, void* pUserData, sml::Agent* pAgent, sml::smlPhase phase); - static const char *MyRhsFunctionHandler(sml::smlRhsEventId id, void* pUserData, sml::Agent* pAgent, char const* pFunctionName, char const* pArgument, int *buffSize, char *buff); + static std::string MyRhsFunctionHandler(sml::smlRhsEventId id, void* pUserData, sml::Agent* pAgent, char const* pFunctionName, char const* pArgument); static const sml::RhsEventHandlerCpp GetRhsFunctionHandlerCpp(bool* receivedFlag); static void MyMemoryLeakUpdateHandlerDestroyChildren(sml::smlUpdateEventId id, void* pUserData, sml::Kernel* pKernel, sml::smlRunFlags runFlags); static void MyMemoryLeakUpdateHandler(sml::smlUpdateEventId id, void* pUserData, sml::Kernel* pKernel, sml::smlRunFlags runFlags); @@ -46,8 +46,8 @@ class Handlers static void MyAgentCreationUpdateEventHandler(sml::smlUpdateEventId id, void* pUserData, sml::Kernel* pKernel, sml::smlRunFlags runFlags); static void MyOrderingPrintHandler(sml::smlPrintEventId id, void* pUserData, sml::Agent* pAgent, char const* pMessage); static void MyOrderingRunHandler(sml::smlRunEventId id, void* pUserData, sml::Agent* pAgent, sml::smlPhase phase); - static const char *MyRhsFunctionFailureHandler(sml::smlRhsEventId id, void* pUserData, sml::Agent* pAgent, char const* pFunctionName, char const* pArgument, int *buffSize, char *buff); - static const char *MySuccessHandler(sml::smlRhsEventId id, void* pUserData, sml::Agent* pAgent, char const* pFunctionName, char const* pArgument, int *buffSize, char *buff); + static std::string MyRhsFunctionFailureHandler(sml::smlRhsEventId id, void* pUserData, sml::Agent* pAgent, char const* pFunctionName, char const* pArgument); + static std::string MySuccessHandler(sml::smlRhsEventId id, void* pUserData, sml::Agent* pAgent, char const* pFunctionName, char const* pArgument); private: static void MyMemoryLeakUpdateHandlerInternal(bool destroyAll, sml::smlUpdateEventId id, void* pUserData, sml::Kernel* pKernel, sml::smlRunFlags runFlags);