RDKB-65730: Integrate Dynamic Table Support - Build Time Disabled

[yogeswaransky]

No description provided.

[Copilot]

Pull request overview

Adds a build-time toggle for Dynamic Table (TR-181 table traversal) support, defaulting the feature to disabled, and gates the related code paths behind ENABLE_DYNAMIC_TABLE_SUPPORT.

Changes:

• Introduces --enable-dynamic-table-support in configure.ac, defining -DENABLE_DYNAMIC_TABLE_SUPPORT when enabled.
• Wraps Dynamic Table structs/APIs and related logic in #ifdef ENABLE_DYNAMIC_TABLE_SUPPORT across parser/reportgen/profile/util code.
• Adjusts runtime behavior to ignore dataModelTable parameters when the feature is disabled.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
configure.ac	Adds configure-time option and defines ENABLE_DYNAMIC_TABLE_SUPPORT macro via CPPFLAGS.
source/utils/t2common.h	Conditionally exposes DataModelParam/DataModelTable and related APIs only when enabled.
source/utils/t2common.c	Conditionally compiles Dynamic Table free helpers and matchesParameter().
source/t2parser/t2parser.c	Conditionally compiles DataModelTable parsing helpers and ignores dataModelTable when disabled.
source/reportgen/reportgen.c	Conditionally compiles Dynamic Table JSON encoding helpers/branch.
source/bulkdata/profile.h	Makes Profile::dataModelTableList conditional on the feature flag.
source/bulkdata/profile.c	Conditionally frees/uses dataModelTableList when enabled.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

configure.ac:239

• With dynamic table support disabled by default, build-time hiding of DataModelTable/Profile::dataModelTableList causes the existing *_dynamictable_gtest.bin test sources (e.g., source/test/t2parser/t2parser_dynamictable_Test.cpp, source/test/reportgen/reportgen_dynamictable_Test.cpp) to fail to compile because they reference these symbols without #ifdef guards. The new AM_CONDITIONAL is defined here but those test binaries are still built unconditionally in their Makefile.am files; they should be wrapped in if ENABLE_DYNAMIC_TABLE_SUPPORT (or the test sources should be made to compile/skip like profile_dynamictable_Test.cpp).

ENABLE_DYNAMIC_TABLE_SUPPORT=false
AC_ARG_ENABLE([dynamic-table-support],
        AS_HELP_STRING([--enable-dynamic-table-support],[enable dynamic TR-181 table traversal support (default is no)]),
        [
          case "${enableval}" in
           yes) ENABLE_DYNAMIC_TABLE_SUPPORT=true ;;
           no)  ENABLE_DYNAMIC_TABLE_SUPPORT=false ;;
          *) AC_MSG_ERROR([bad value ${enableval} for --enable-dynamic-table-support]) ;;
           esac
           ],
        [echo "dynamic table support is disabled"])
AM_CONDITIONAL([ENABLE_DYNAMIC_TABLE_SUPPORT], [test x$ENABLE_DYNAMIC_TABLE_SUPPORT = xtrue])

if test x$ENABLE_DYNAMIC_TABLE_SUPPORT = xtrue; then
    CPPFLAGS="$CPPFLAGS -DENABLE_DYNAMIC_TABLE_SUPPORT"
fi

AC_MSG_NOTICE([Dynamic table support: $ENABLE_DYNAMIC_TABLE_SUPPORT])

source/test/mocks/profileStub.c:115

• When dynamic table support is enabled, dataModelTableList elements are DataModelTable allocations (with nested paramList). Destroying the vector with a NULL element free function leaks those allocations and can break valgrind-based tests; use freeDataModelTable here to match production cleanup.

    if(profile->dataModelTableList)
    {
        Vector_Destroy(profile->dataModelTableList, NULL);
    }

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

configure.ac:251

• With dynamic table support disabled by default, several gtest targets still build and compile dynamic-table tests unconditionally. For example, source/test/reportgen/reportgen_dynamictable_Test.cpp and source/test/t2parser/t2parser_dynamictable_Test.cpp use DataModelTable/matchesParameter without any ENABLE_DYNAMIC_TABLE_SUPPORT guards, and their Makefile.am files always include the corresponding *_dynamictable_gtest.bin programs. Since DataModelTable/matchesParameter are now only defined when ENABLE_DYNAMIC_TABLE_SUPPORT is set (source/utils/t2common.h), the default build is likely to fail when tests are enabled unless those test binaries are conditionally built or compiled with -DENABLE_DYNAMIC_TABLE_SUPPORT.

AM_CONDITIONAL([ENABLE_DYNAMIC_TABLE_SUPPORT], [test x$ENABLE_DYNAMIC_TABLE_SUPPORT = xtrue])

if test x$ENABLE_DYNAMIC_TABLE_SUPPORT = xtrue; then
    CPPFLAGS="$CPPFLAGS -DENABLE_DYNAMIC_TABLE_SUPPORT"
fi

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

source/bulkdata/profile.h:99

• When ENABLE_DYNAMIC_TABLE_SUPPORT is off, Profile no longer has dataModelTableList. Some always-built tests (e.g., source/test/t2parser/t2parser_dynamictable_Test.cpp references profile->dataModelTableList at ~521) will not compile in the default build. The dynamic-table test binaries should be built only when ENABLE_DYNAMIC_TABLE_SUPPORT is enabled, or the tests should be guarded to skip when disabled.

    Vector *cachedReportList;
#ifdef ENABLE_DYNAMIC_TABLE_SUPPORT
    Vector *dataModelTableList;   // List of DataModelTable
#endif

source/utils/t2common.h:156

• With ENABLE_DYNAMIC_TABLE_SUPPORT disabled (default), DataModelParam/DataModelTable are not defined. However, the dynamic-table gtest binaries are built unconditionally (e.g., source/test/reportgen/Makefile.am builds reportgen_dynamictable_gtest.bin and reportgen_dynamictable_Test.cpp uses DataModelTable at lines ~149+), which will now fail to compile. Either conditionally build these *_dynamictable_gtest.bin targets using the new AM_CONDITIONAL, or wrap those tests with #ifdef/GTEST_SKIP like bulkdata/profile_dynamictable_Test.cpp.

#ifdef ENABLE_DYNAMIC_TABLE_SUPPORT
typedef struct _DataModelParam
{
    char *name;
    char *reference;
    bool reportEmpty;
} DataModelParam;

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

source/test/mocks/profileStub.c:115

• When ENABLE_DYNAMIC_TABLE_SUPPORT is enabled, this stub destroys dataModelTableList without freeing DataModelTable elements, which can leak heap allocations in dynamic-table unit tests (and can mask issues under valgrind). Prefer using the real element destructor used in production code (freeDataModelTable).

#ifdef ENABLE_DYNAMIC_TABLE_SUPPORT
    if(profile->dataModelTableList)
    {
        Vector_Destroy(profile->dataModelTableList, NULL);
    }

[Copilot]

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

source/test/mocks/profileStub.c:115

• In the test stub, dataModelTableList elements are heap-allocated (DataModelTable with nested paramList), but freeProfile() destroys the vector with a NULL element destructor. With ENABLE_DYNAMIC_TABLE_SUPPORT enabled in the dynamic-table test binaries, this leaks the DataModelTable objects and their nested DataModelParam allocations, and diverges from production freeProfile() (source/bulkdata/profile.c uses freeDataModelTable).

#ifdef ENABLE_DYNAMIC_TABLE_SUPPORT
    if(profile->dataModelTableList)
    {
        Vector_Destroy(profile->dataModelTableList, NULL);
    }

[Copilot]

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

source/test/reportgen/reportgen_dynamictable_Test.cpp:843

• Cleanup in this test manually frees Vector elements as if they were char*/tr181ValStruct_t*, but encodeParamResultInJSON() expects Param*/profileValues* and the setup should match that contract. Once the vectors contain Param/profileValues, use the existing free helpers to avoid leaks/double-frees and keep cleanup consistent with other reportgen tests (e.g., source/test/reportgen/reportgenTest.cpp:759-761).

    // Cleanup
    cJSON_Delete(valArray);
    for (int i = 0; i < 2; i++) {
        free((char*)Vector_At(paramNameList, i));
        tr181ValStruct_t* pv = (tr181ValStruct_t*)Vector_At(paramValueList, i);
        free(pv->parameterName);
        free(pv->parameterValue);
        free(pv);
    }
    Vector_Destroy(paramNameList, NULL);
    Vector_Destroy(paramValueList, NULL);

source/test/t2parser/t2parser_dynamictable_Test.cpp:1120

• The new nested dynamic-table test never reaches DataModelTable parsing because the JSON profile is missing required fields for processConfiguration() (e.g., "Hash" and the required "HTTP" and "JSONEncoding.ReportFormat" sections). As written, processConfiguration() returns T2ERROR_FAILURE during profile validation, so the assertions on dataModelTableList will be skipped and the test will pass without exercising the feature.

    const char* nestedTableConfig = R"({
        "Description": "Nested Table Test",
        "Version": "1",
        "Protocol": "HTTP",
        "EncodingType": "JSON",
        "ReportingInterval": 60,

[Copilot]

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (3)

source/reportgen/reportgen.c:303

• When dynamic table support is enabled, this file uses isdigit() (e.g., in getBasePath / findTableByReference / encodeParamResultInJSON) but does not include <ctype.h>. This can fail to compile on toolchains that treat implicit declarations as errors and is undefined behavior in modern C. Add <ctype.h> in this compilation unit (ideally under the ENABLE_DYNAMIC_TABLE_SUPPORT guard, since it’s only needed for that code path).

#ifdef ENABLE_DYNAMIC_TABLE_SUPPORT
//Function to get the basePath like Device.WiFi.AccessPoint.
int getBasePath(const char *input, char *basePath, size_t maxLength)

source/test/mocks/profileStub.c:115

• freeProfile() in this stub destroys dataModelTableList with a NULL element destructor. With ENABLE_DYNAMIC_TABLE_SUPPORT enabled, t2parser/reportgen tests populate this vector with heap-allocated DataModelTable/DataModelParam objects, so this leaks memory and can mask regressions in the memory-safety test suite. Use freeDataModelTable as the Vector_Destroy cleanup function under ENABLE_DYNAMIC_TABLE_SUPPORT.

#ifdef ENABLE_DYNAMIC_TABLE_SUPPORT
    if(profile->dataModelTableList)
    {
        Vector_Destroy(profile->dataModelTableList, NULL);
    }

source/test/reportgen/reportgen_dynamictable_Test.cpp:813

• This test constructs paramNameList as a Vector of char* and paramValueList as a Vector of tr181ValStruct_t*. However encodeParamResultInJSON() expects paramNameList elements to be Param* and paramValueList elements to be profileValues* (see reportgen.c: Param* param = (Param*)Vector_At(...); profileValues* pv = (profileValues*)Vector_At(...)). With the current types, the test invokes undefined behavior and is likely to crash or produce meaningless coverage. Update the test helpers to build real Param/profileValues objects (or provide a thin adapter wrapper) before calling encodeParamResultInJSON.

    for (int i = 0; i < 2; i++) {
        char* pName = strdup(params[i]);
        Vector_PushBack(paramNameList, pName);

        tr181ValStruct_t* pVal = (tr181ValStruct_t*)malloc(sizeof(tr181ValStruct_t));
        pVal->parameterName = strdup(params[i]);
        pVal->parameterValue = strdup(values[i]);
        Vector_PushBack(paramValueList, pVal);