From 5b6a4151ed2cc7dc58e1335877040f2519d3ea38 Mon Sep 17 00:00:00 2001 From: SGM-BonDeVoyage Date: Fri, 21 Jun 2024 09:32:26 +0000 Subject: [PATCH] RDKBDEV-2687 tableSyncHandler callback to avoid undefined behavior while unregistering a row --- include/rbus.h | 66 +++++++++++++++++- src/rbus/rbus.c | 149 ++++++++++++++++++++++++++++------------ src/rbus/rbus_element.c | 36 +++++++--- src/rbus/rbus_element.h | 3 +- 4 files changed, 199 insertions(+), 55 deletions(-) diff --git a/include/rbus.h b/include/rbus.h index 41dcaa42..aeaa8d09 100644 --- a/include/rbus.h +++ b/include/rbus.h @@ -530,7 +530,24 @@ typedef rbusError_t (* rbusEventSubHandler_t)( bool* autoPublish ); -/** @struct rbusCallbackTable_t +/** @fn typedef rbusError_t (*rbusTableSyncHandler_t)( + * rbusHandle_t handle, + * char const* tableName) + * @brief A table sync callback handler + * + * A provider can implement this handler to allow dynamic tables to synchronize rows. + * The tableName parameter will be a fully qualified name, specifying table's name + * (e.g. "Device.IP.Interface."). + * @param handle Bus Handle + * @param tableName The name of a table (e.g. "Device.IP.Interface.") + * @return RBus error code as defined by rbusError_t. + */ +typedef rbusError_t (*rbusTableSyncHandler_t)( + rbusHandle_t handle, + char const* tableName +); + +/** @struct rbusElementCallbackTable_t * @brief The list of callback handlers supported by a data element. * * This table also specifies the possible usage for each data element. @@ -549,7 +566,7 @@ typedef rbusError_t (* rbusEventSubHandler_t)( * data element, the rbus library checks for NULL and substitutes a pointer * to an error handler function for all unused features */ -typedef struct rbusCallbackTable_t +typedef struct rbusElementCallbackTable_t { rbusGetHandler_t getHandler; /**< Get parameters handler for the @@ -566,6 +583,32 @@ typedef struct rbusCallbackTable_t handler for the event name */ rbusMethodHandler_t methodHandler; /**< Method handler */ + rbusTableSyncHandler_t tableSyncHandler; /** Synchronize dynamic table */ +} rbusElementCallbackTable_t; + +/** + * @struct rbusCallbackTable_t + * @brief Backward compatibility version of rbusElementCallbackTable_t + */ +typedef struct rbusCallbackTable_t +{ + rbusGetHandler_t getHandler; /**< Get parameters + handler for the + named paramter */ + rbusSetHandler_t setHandler; /**< Set parameters + handler for the + named parameter */ + rbusTableAddRowHandler_t tableAddRowHandler; /**< Add row handler + to a table*/ + rbusTableRemoveRowHandler_t tableRemoveRowHandler; /**< Remove a row + from a table*/ + rbusEventSubHandler_t eventSubHandler; /**< Event subscribe + and unsubscribe + handler for the + event name */ + void* methodHandler; /**< Method handler. + For table type elements (RBUS_ELEMENT_TYPE_TABLE) method handler + is used to synchronize table rows of dynamic tables */ } rbusCallbackTable_t; /// @brief rbusDataElement_t The structure used when registering or @@ -1898,6 +1941,25 @@ rbusError_t rbus_openDirect(rbusHandle_t handle, rbusHandle_t* myDirectHandle, c * @return RBus error code as defined by rbusError_t. */ rbusError_t rbus_closeDirect(rbusHandle_t handle); + +/** @fn rbusError_t rbus_registerDynamicTableSyncHandler( + * rbusHandle_t handle, + * char const* tableName, + * rbusTableSyncHandler_t syncHandler) + * + * @brief Register tableSyncHandler for table element with dynamic rows. + * + * Used by: Component that wants to register tableSyncHandler that will register/unregister rows of dynamic table. + * + * @param handle Bus Handle + * @param tableName The name of a table (e.g. "Device.IP.Interface.") + * @param syncHandler Dymanic table sync callback + * @return RBus error code as defined by rbusError_t. + */ +rbusError_t rbus_registerDynamicTableSyncHandler( + rbusHandle_t handle, + char const* tableName, + rbusTableSyncHandler_t syncHandler); /** @} */ #ifdef __cplusplus diff --git a/src/rbus/rbus.c b/src/rbus/rbus.c index a12965e1..720cbd27 100644 --- a/src/rbus/rbus.c +++ b/src/rbus/rbus.c @@ -1452,7 +1452,7 @@ static void _set_callback_handler (rbusHandle_t handle, rbusMessage request, rbu { /* Retrive the element node */ char const* paramName = rbusProperty_GetName(pProperties[loopCnt]); - el = retrieveInstanceElement(handleInfo->elementRoot, paramName); + el = retrieveInstanceElementEx(handle, handleInfo->elementRoot, paramName, true); if(el != NULL) { if(el->cbTable.setHandler) @@ -1613,46 +1613,55 @@ static void _get_recursive_partialpath_handler(elementNode* node, char const* qu if (node != NULL) { - /*if table getHandler, then pass the query to it and stop recursion*/ - if(node->type == RBUS_ELEMENT_TYPE_TABLE && node->cbTable.getHandler) + if(node->type == RBUS_ELEMENT_TYPE_TABLE) { - rbusError_t result; - rbusProperty_t tmpProperties; - char instanceName[RBUS_MAX_NAME_LENGTH]; - char partialPath[RBUS_MAX_NAME_LENGTH]; + /*if table getHandler, then pass the query to it and stop recursion*/ + if (node->cbTable.getHandler) + { + rbusError_t result; + rbusProperty_t tmpProperties; + char instanceName[RBUS_MAX_NAME_LENGTH]; + char partialPath[RBUS_MAX_NAME_LENGTH]; - snprintf(partialPath, RBUS_MAX_NAME_LENGTH-1, "%s.", - query ? _convert_reg_name_to_instance_name(node->fullName, query, instanceName) : node->fullName); + snprintf(partialPath, RBUS_MAX_NAME_LENGTH-1, "%s.", + query ? _convert_reg_name_to_instance_name(node->fullName, query, instanceName) : node->fullName); - RBUSLOG_DEBUG("%*s_get_recursive_partialpath_handler calling table getHandler partialPath=%s", level*4, " ", partialPath); + RBUSLOG_DEBUG("%*s_get_recursive_partialpath_handler calling table getHandler partialPath=%s", level*4, " ", partialPath); - rbusProperty_Init(&tmpProperties, partialPath, NULL); + rbusProperty_Init(&tmpProperties, partialPath, NULL); - ELM_PRIVATE_LOCK(node); - result = node->cbTable.getHandler(handle, tmpProperties, &options); - ELM_PRIVATE_UNLOCK(node); + ELM_PRIVATE_LOCK(node); + result = node->cbTable.getHandler(handle, tmpProperties, &options); + ELM_PRIVATE_UNLOCK(node); - if (result == RBUS_ERROR_SUCCESS ) - { - int count = rbusProperty_Count(tmpProperties); + if (result == RBUS_ERROR_SUCCESS ) + { + int count = rbusProperty_Count(tmpProperties); - RBUSLOG_DEBUG("%*s_get_recursive_partialpath_handler table getHandler returned %d properties", level*4, " ", count-1); + RBUSLOG_DEBUG("%*s_get_recursive_partialpath_handler table getHandler returned %d properties", level*4, " ", count-1); - /*the first property is just the partialPath we passed in */ - if(count > 1) + /*the first property is just the partialPath we passed in */ + if(count > 1) + { + /*take the second property, which is a list*/ + rbusProperty_Append(properties, rbusProperty_GetNext(tmpProperties)); + *pCount += count - 1; + } + } + else { - /*take the second property, which is a list*/ - rbusProperty_Append(properties, rbusProperty_GetNext(tmpProperties)); - *pCount += count - 1; + RBUSLOG_DEBUG("%*s_get_recursive_partialpath_handler table getHandler failed rc=%d", level*4, " ", result); } + + rbusProperty_Release(tmpProperties); + return; } - else + else if (node->cbTable.tableSyncHandler) { - RBUSLOG_DEBUG("%*s_get_recursive_partialpath_handler table getHandler failed rc=%d", level*4, " ", result); + ELM_PRIVATE_LOCK(node); + node->cbTable.tableSyncHandler(handle, node->fullName); + ELM_PRIVATE_UNLOCK(node); } - - rbusProperty_Release(tmpProperties); - return; } elementNode* child = node->child; @@ -1726,13 +1735,20 @@ rbusError_t get_recursive_wildcard_handler (rbusHandle_t handle, char const *par tmpPtr[0] = '\0'; tmpPtr++; - el = retrieveInstanceElement(handleInfo->elementRoot, instanceName); + el = retrieveInstanceElementEx(handle, handleInfo->elementRoot, instanceName, true); if (!el) return RBUS_ERROR_ELEMENT_DOES_NOT_EXIST; else if(el->type != RBUS_ELEMENT_TYPE_TABLE) return RBUS_ERROR_ACCESS_NOT_ALLOWED; + if (el->cbTable.tableSyncHandler) + { + ELM_PRIVATE_LOCK(el); + el->cbTable.tableSyncHandler(handle, el->fullName); + ELM_PRIVATE_UNLOCK(el); + } + child = el->child; while(child) { @@ -1752,7 +1768,7 @@ rbusError_t get_recursive_wildcard_handler (rbusHandle_t handle, char const *par else if (instanceName[length] == '.') { int hasInstance = 1; - el = retrieveInstanceElement(handleInfo->elementRoot, instanceName); + el = retrieveInstanceElementEx(handle, handleInfo->elementRoot, instanceName, true); if(el) { if(strstr(el->fullName, "{i}")) @@ -1764,7 +1780,7 @@ rbusError_t get_recursive_wildcard_handler (rbusHandle_t handle, char const *par } else { - child = retrieveInstanceElement(handleInfo->elementRoot, instanceName); + child = retrieveInstanceElementEx(handle, handleInfo->elementRoot, instanceName, true); if (!child) return RBUS_ERROR_ELEMENT_DOES_NOT_EXIST; @@ -1800,7 +1816,7 @@ static rbusError_t _get_single_dml_handler (rbusHandle_t handle, char const *par RBUSLOG_DEBUG("calling get single for [%s]", parameterName); - el = retrieveInstanceElement(handleInfo->elementRoot, parameterName); + el = retrieveInstanceElementEx(handle, handleInfo->elementRoot, parameterName, true); if(el != NULL) { RBUSLOG_DEBUG("Retrieved [%s]", parameterName); @@ -1952,7 +1968,7 @@ static void _get_callback_handler (rbusHandle_t handle, rbusMessage request, rbu return; } -static void _get_parameter_names_recurse(elementNode* el, int* count, rbusMessage response, int requestedDepth, int currentDepth) +static void _get_parameter_names_recurse(rbusHandle_t handle, elementNode* el, int* count, rbusMessage response, int requestedDepth, int currentDepth, bool syncTables) { int absDepth = abs(requestedDepth); @@ -2003,13 +2019,22 @@ static void _get_parameter_names_recurse(elementNode* el, int* count, rbusMessag if(currentDepth < absDepth) { - elementNode* child = el->child; + elementNode* child; + + if (syncTables && el->type == RBUS_ELEMENT_TYPE_TABLE && el->cbTable.tableSyncHandler) + { + ELM_PRIVATE_LOCK(el); + el->cbTable.tableSyncHandler(handle, el->fullName); + ELM_PRIVATE_UNLOCK(el); + } + + child = el->child; while(child) { if( !(child->type == RBUS_ELEMENT_TYPE_TABLE && child->cbTable.getHandler) && /*TODO table with get handler */ !(el->type == RBUS_ELEMENT_TYPE_TABLE && strcmp(child->name, "{i}") == 0))/*if not a table row template*/ { - _get_parameter_names_recurse(child, count, response, requestedDepth, currentDepth+1); + _get_parameter_names_recurse(handle, child, count, response, requestedDepth, currentDepth+1, syncTables); } child = child->nextSibling; } @@ -2037,7 +2062,7 @@ static void _get_parameter_names_handler (rbusHandle_t handle, rbusMessage reque rbusMessage_Init(response); - el = retrieveInstanceElement(handleInfo->elementRoot, objName); + el = retrieveInstanceElementEx(handle, handleInfo->elementRoot, objName, true); if (!el) { rbusMessage_SetInt32(*response, (int)RBUS_ERROR_ELEMENT_DOES_NOT_EXIST); @@ -2046,7 +2071,7 @@ static void _get_parameter_names_handler (rbusHandle_t handle, rbusMessage reque if(getRowNamesOnly) { - elementNode* child = el->child; + elementNode* child; int numRows = 0; if(el->type != RBUS_ELEMENT_TYPE_TABLE) @@ -2054,6 +2079,14 @@ static void _get_parameter_names_handler (rbusHandle_t handle, rbusMessage reque rbusMessage_SetInt32(*response, (int)RBUS_ERROR_INVALID_INPUT); return; } + else if (el->cbTable.tableSyncHandler) + { + ELM_PRIVATE_LOCK(el); + el->cbTable.tableSyncHandler(handle, el->fullName); + ELM_PRIVATE_UNLOCK(el); + } + + child = el->child; while(child) { @@ -2087,13 +2120,13 @@ static void _get_parameter_names_handler (rbusHandle_t handle, rbusMessage reque rbusMessage_SetInt32(*response, RBUS_ERROR_SUCCESS); - _get_parameter_names_recurse(el, &count, NULL, requestedDepth, 0); + _get_parameter_names_recurse(handle, el, &count, NULL, requestedDepth, 0, true); RBUSLOG_DEBUG("found %d elements", count); rbusMessage_SetInt32(*response, (int)count); - _get_parameter_names_recurse(el, NULL, *response, requestedDepth, 0); + _get_parameter_names_recurse(handle, el, NULL, *response, requestedDepth, 0, false); } #if 0 //TODO-finish else /*if table with getHandler*/ @@ -2172,7 +2205,7 @@ static void _table_add_row_callback_handler (rbusHandle_t handle, rbusMessage re RBUSLOG_DEBUG("table [%s] alias [%s] name [%s]", tableName, aliasName, handleInfo->componentName); elementNode* tableRegElem = retrieveElement(handleInfo->elementRoot, tableName); - elementNode* tableInstElem = retrieveInstanceElement(handleInfo->elementRoot, tableName); + elementNode* tableInstElem = retrieveInstanceElementEx(handle, handleInfo->elementRoot, tableName, true); if(tableRegElem && tableInstElem) { @@ -2224,7 +2257,7 @@ static void _table_remove_row_callback_handler (rbusHandle_t handle, rbusMessage /*get the element for the row */ elementNode* rowRegElem = retrieveElement(handleInfo->elementRoot, rowName); - elementNode* rowInstElem = retrieveInstanceElement(handleInfo->elementRoot, rowName); + elementNode* rowInstElem = retrieveInstanceElementEx(handle, handleInfo->elementRoot, rowName, true); if(rowRegElem && rowInstElem) { @@ -2304,7 +2337,7 @@ static int _method_callback_handler(rbusHandle_t handle, rbusMessage request, rb rbusObject_Init(&outParams, NULL); /*get the element for the row */ elementNode* methRegElem = retrieveElement(handleInfo->elementRoot, methodName); - elementNode* methInstElem = retrieveInstanceElement(handleInfo->elementRoot, methodName); + elementNode* methInstElem = retrieveInstanceElementEx(handle, handleInfo->elementRoot, methodName, true); if(methRegElem && methInstElem) { @@ -2431,7 +2464,7 @@ static void _subscribe_callback_handler (rbusHandle_t handle, rbusMessage reques RBUSLOG_ERROR("payload missing in subscribe request for event %s from %s", event_name, sender); } - el = retrieveInstanceElement(handleInfo->elementRoot, event_name); + el = retrieveInstanceElementEx(handle, handleInfo->elementRoot, event_name, true); if (!el) { @@ -2578,7 +2611,7 @@ static void _create_direct_connection_callback_handler (rbusHandle_t handle, rbu rbusMessage_Init(response); - el = retrieveInstanceElement(handleInfo->elementRoot, paramName); + el = retrieveInstanceElementEx(handle, handleInfo->elementRoot, paramName, true); if (el) { @@ -6115,4 +6148,32 @@ rbusError_t rbusHandle_GetTraceContextAsString( return RBUS_ERROR_SUCCESS; } +rbusError_t rbus_registerDynamicTableSyncHandler( + rbusHandle_t handle, + char const* tableName, + rbusTableSyncHandler_t syncHandler) +{ + struct _rbusHandle* handleInfo; + elementNode* tableRegElem; + + VERIFY_HANDLE(handle); + VERIFY_NULL(tableName); + + handleInfo = (struct _rbusHandle*)handle; + tableRegElem = retrieveElement(handleInfo->elementRoot, tableName); + + if (!tableRegElem) + return RBUS_ERROR_ELEMENT_DOES_NOT_EXIST; + + if(!strcmp(tableRegElem->name, "{i}")) + tableRegElem = tableRegElem->parent; + + if (tableRegElem->type != RBUS_ELEMENT_TYPE_TABLE) + return RBUS_ERROR_INVALID_INPUT; + + tableRegElem->cbTable.tableSyncHandler = syncHandler; + + return RBUS_ERROR_SUCCESS; +} + /* End of File */ diff --git a/src/rbus/rbus_element.c b/src/rbus/rbus_element.c index a404c03c..8658af1c 100644 --- a/src/rbus/rbus_element.c +++ b/src/rbus/rbus_element.c @@ -236,7 +236,7 @@ elementNode* insertElement(elementNode* root, rbusDataElement_t* elem) if(!mutex_init) { ERROR_CHECK(pthread_mutexattr_init(&attrib)); - ERROR_CHECK(pthread_mutexattr_settype(&attrib, PTHREAD_MUTEX_ERRORCHECK)); + ERROR_CHECK(pthread_mutexattr_settype(&attrib, PTHREAD_MUTEX_RECURSIVE)); ERROR_CHECK(pthread_mutex_init(&element_mutex, &attrib)); mutex_init = 1; } @@ -343,7 +343,11 @@ elementNode* insertElement(elementNode* root, rbusDataElement_t* elem) if(ret == 0) { currentNode->type = elem->type; - currentNode->cbTable = elem->cbTable; + currentNode->cbTable.getHandler = elem->cbTable.getHandler; + currentNode->cbTable.setHandler = elem->cbTable.setHandler; + currentNode->cbTable.tableAddRowHandler = elem->cbTable.tableAddRowHandler; + currentNode->cbTable.tableRemoveRowHandler = elem->cbTable.tableRemoveRowHandler; + currentNode->cbTable.eventSubHandler = elem->cbTable.eventSubHandler; /* See the big comment near the top of this function. We add {i} as a child object of the table. @@ -359,6 +363,11 @@ elementNode* insertElement(elementNode* root, rbusDataElement_t* elem) snprintf(buff, RBUS_MAX_NAME_LENGTH, "%s.%s", currentNode->fullName, rowTemplate->name); rowTemplate->fullName = strdup(buff); currentNode->child = rowTemplate; + currentNode->cbTable.tableSyncHandler = elem->cbTable.methodHandler; + } + else + { + currentNode->cbTable.methodHandler = elem->cbTable.methodHandler; } } free(name); @@ -472,6 +481,11 @@ elementNode* retrieveElement(elementNode* root, const char* elmentName) } elementNode* retrieveInstanceElement(elementNode* root, const char* elmentName) +{ + return retrieveInstanceElementEx(NULL, root, elmentName, false); +} + +elementNode* retrieveInstanceElementEx(rbusHandle_t handle, elementNode* root, const char* elmentName, bool syncTables) { char* token = NULL; char* name = NULL; @@ -510,8 +524,6 @@ elementNode* retrieveInstanceElement(elementNode* root, const char* elmentName) { RBUSLOG_DEBUG("tokenFound!"); tokenFound = 1; - currentNode = nextNode; - nextNode = currentNode->child; } else { @@ -525,8 +537,6 @@ elementNode* retrieveInstanceElement(elementNode* root, const char* elmentName) { RBUSLOG_DEBUG("tokenFound!"); tokenFound = 1; - currentNode = nextNode; - nextNode = currentNode->child; break; } else @@ -543,8 +553,6 @@ elementNode* retrieveInstanceElement(elementNode* root, const char* elmentName) { RBUSLOG_DEBUG("tokenFound by alias %s!", nextNode->alias); tokenFound = 1; - currentNode = nextNode; - nextNode = currentNode->child; break; } } @@ -559,6 +567,18 @@ elementNode* retrieveInstanceElement(elementNode* root, const char* elmentName) token = strtok_r(NULL, ".", &saveptr); + if (tokenFound) + { + if (token && nextNode->type == RBUS_ELEMENT_TYPE_TABLE && (syncTables && nextNode->cbTable.tableSyncHandler)) + { + ELM_PRIVATE_LOCK(nextNode); + nextNode->cbTable.tableSyncHandler(handle, nextNode->fullName); + ELM_PRIVATE_UNLOCK(nextNode); + } + currentNode = nextNode; + nextNode = currentNode->child; + } + if(token && nextNode && nextNode->parent && nextNode->parent->type == RBUS_ELEMENT_TYPE_TABLE) { if(!isWildcard && !strcmp(token,"*")) diff --git a/src/rbus/rbus_element.h b/src/rbus/rbus_element.h index ce8b3834..dfef11bf 100644 --- a/src/rbus/rbus_element.h +++ b/src/rbus/rbus_element.h @@ -66,7 +66,7 @@ typedef struct elementNode elementNode* nextSibling; /* Right */ elementNode* nextPrune; rbusElementType_t type; /* Type w/ Object=0 */ - rbusCallbackTable_t cbTable; /* Callback table for the element */ + rbusElementCallbackTable_t cbTable; /* Callback table for the element */ rtList subscriptions; /* The list of rbusSubscription_t to this element */ char* alias; /* For table rows */ char* changeComp; /* For properties, the last component to set the value */ @@ -82,6 +82,7 @@ elementNode* insertElement(elementNode* root, rbusDataElement_t* elem); void removeElement(elementNode* element); elementNode* retrieveElement(elementNode* root, const char* name); elementNode* retrieveInstanceElement(elementNode* root, const char* name); +elementNode* retrieveInstanceElementEx(rbusHandle_t handle, elementNode* root, const char* name, bool syncTables); void printRegisteredElements(elementNode* root, int level); void fprintRegisteredElements(FILE* f, elementNode* root, int level); void addElementSubscription(elementNode* node, rbusSubscription_t* sub, bool checkIfExists);