RDKEMW-8587: consume the config variables using dlsym() in MW by yuvaramachandran-gurusamy · Pull Request #185 · rdkcentral/devicesettings

yuvaramachandran-gurusamy · 2025-12-11T14:56:18Z

RDKEMW-8587: consume the config variables using dlsym() in MW.

Reason for change: consume the config variables using dlsym() in MW.
Test Procedure: refer RDKEMW-8587
Risks: High
Signed-off-by:gsanto722 grandhi_santoshkumar@comcast.com

ds/videoDeviceConfig.cpp

ds/videoOutputPortConfig.cpp

ds/audioOutputPortConfig.cpp

ds/videoOutputPortConfig.cpp

apatel859 · 2025-12-13T06:21:22Z

ds/frontPanelConfig.cpp

+    int indicatorColorSize = -1;
+    int textDisplaySize = (configuration->pKTextDisplays_size) ? *(configuration->pKTextDisplays_size) : -1;
+    // Dump the configuration details
+    INT_INFO("\n\n===========================================================================\n\n");


this two line be comboned like

INT_INFO("\n===========Starting to Dump FrontPanel Configs=======\n");

apatel859 · 2025-12-13T06:22:02Z

ds/frontPanelConfig.cpp

 #include "dslogger.h"
+#include "manager.hpp"
+
+#define DEBUG 1 // Using for dumpconfig 


if you already have access("/opt/dsMgrDumpDeviceConfigs", chec kwhy we need DEBUG 1?

apatel859 · 2025-12-13T06:24:49Z

ds/videoOutputPortConfig.cpp

+            INT_INFO("resolution->interlaced= %d", resolution->interlaced);
+        }
+        INT_INFO("\n\n####################################################################### \n\n");
+        INT_INFO("\n\n####################################################################### \n\n");


reduce unnecessary logs every where

apatel859

i see excess logs can u reduce as commented in all placeses

Copilot

Pull request overview

This PR implements dynamic configuration loading using dlsym() for the device settings middleware layer, enabling runtime loading of device capabilities from shared libraries instead of static compile-time configuration.

Key Changes:

Introduced loadDeviceCapabilities() function in manager.cpp that uses dlsym() to dynamically load configuration symbols from RDK_DSHAL_NAME library
Modified load() methods across all config classes (Audio/Video/FrontPanel) to accept configuration structure pointers, supporting both dynamic and static configurations
Enhanced logging infrastructure by adding function name to log macros and replacing printf statements with INT_INFO/INT_ERROR for consistency
Added dumpconfig() functions for debugging configuration data when /opt/dsMgrDumpDeviceConfigs file exists

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 25 comments.

Show a summary per file

File	Description
ds/videoOutputPortConfig.hpp	Added videoPortConfigs_t struct and modified load() signature to accept config pointer
ds/videoOutputPortConfig.cpp	Implemented dynamic config loading with fallback to static, added dumpconfig() function, converted printf to INT_INFO
ds/videoDeviceConfig.hpp	Added videoDeviceConfig_t struct and modified load() signature
ds/videoDeviceConfig.cpp	Implemented dynamic config loading with fallback to static, added dumpconfig() function
ds/audioOutputPortConfig.hpp	Added audioConfigs_t struct and modified load() signature
ds/audioOutputPortConfig.cpp	Implemented dynamic config loading with fallback to static, added dumpconfig() function
ds/frontPanelConfig.hpp	Added fpdConfigs_t struct, moved load() to public, added m_isFPConfigLoaded flag
ds/frontPanelConfig.cpp	Implemented dynamic config loading, removed automatic load() call from getInstance(), added dumpconfig()
ds/manager.cpp	Added LoadDLSymbols() and loadDeviceCapabilities() functions, integrated dynamic loading into Initialize() and load()
ds/include/manager.hpp	Added DeviceCapabilityTypes enum, dlSymbolLookup struct, function declarations
ds/include/dslogger.h	Changed LogLevel to enum, added function parameter to ds_log() signature
ds/dslogger.cpp	Enhanced logging with thread ID, function name, and improved formatting
ds/videoOutputPort.cpp	Code formatting changes (indentation) and printf to INT_INFO conversion
ds/host.cpp	Converted printf to INT_INFO
ds/hdmiIn.cpp	Converted printf to INT_INFO throughout
ds/frontPanelIndicator.cpp	Removed empty lines (formatting only)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-13T06:42:17Z

ds/videoDeviceConfig.cpp

 #include "dslogger.h"
+#include "manager.hpp"

+#define DEBUG 1 // Using for dumpconfig 


The 'DEBUG' macro is defined but never used in this file. If it's intended to control the dumpconfig functionality, it should be checked before calling dumpconfig(), or removed if unnecessary. Consider removing it to avoid confusion.

Suggested change

#define DEBUG 1 // Using for dumpconfig

Copilot · 2025-12-13T06:42:18Z

ds/include/manager.hpp

 	static void DeInitialize();
    static void load(); //!< This function is being used for loading configure in-process DSMgr.
-	static int IsInitialized;   //!< Indicates the application has initialized with devicettings modules.
+    static int IsInitialized;   //!< Indicates the application has initialized with devicettings modules.


The comment references 'devicettings' which appears to be a typo for 'device settings'. This typo exists in the original code but should be corrected for clarity.

Suggested change

static int IsInitialized; //!< Indicates the application has initialized with devicettings modules.

static int IsInitialized; //!< Indicates the application has initialized with Device Settings modules.

Copilot · 2025-12-13T06:42:18Z

ds/manager.cpp

 	{std::lock_guard<std::mutex> lock(gManagerInitMutex);
-	printf("Entering %s count %d with thread id %lu\n",__FUNCTION__,IsInitialized,pthread_self());
+
+	INT_INFO("Entering ... count %d with thread id %lu\n",IsInitialized,pthread_self());


The format string at line 230 has an unnecessary '\n' character. The INT_INFO macro already handles line termination as shown in dslogger.cpp line 66-72 where '\n' is added. Remove the '\n' from the format string for consistency.

Suggested change

INT_INFO("Entering ... count %d with thread id %lu\n",IsInitialized,pthread_self());

INT_INFO("Entering ... count %d with thread id %lu",IsInitialized,pthread_self());

Copilot · 2025-12-13T06:42:18Z

ds/frontPanelConfig.cpp

+    }
+    else
+    {
+        INT_ERROR("No valid text display configuration found\n");


The INT_ERROR message at line 507 ends with '\n', but the INT_ERROR macro already adds a newline. This results in extra blank lines in the log output. Remove the '\n' from the format string.

Suggested change

INT_ERROR("No valid text display configuration found\n");

INT_ERROR("No valid text display configuration found");

Copilot · 2025-12-13T06:42:19Z

ds/frontPanelConfig.cpp

+    if (( false == m_isFPInitialized) || (true == m_isFPConfigLoaded)) {
+        INT_ERROR("'%s'", (!m_isFPInitialized) ? "Front Panel not initialized" : "Front Panel Config already loaded");


The function returns early at line 427 if config is already loaded or if FP is not initialized, but the check should ensure that if FP is not initialized, it should return an error or throw an exception. The current logic with return statement may silently fail without notifying the caller of the configuration load failure. Consider throwing an exception instead of returning silently.

Suggested change

if (( false == m_isFPInitialized) || (true == m_isFPConfigLoaded)) {

INT_ERROR("'%s'", (!m_isFPInitialized) ? "Front Panel not initialized" : "Front Panel Config already loaded");

if (!m_isFPInitialized) {

INT_ERROR("'Front Panel not initialized'");

throw IllegalArgumentException("Front Panel not initialized");

}

if (m_isFPConfigLoaded) {

INT_ERROR("'Front Panel Config already loaded'");

Copilot · 2025-12-13T06:42:23Z

ds/frontPanelConfig.cpp

                _singleton.m_isFPInitialized = true;
                INT_INFO("dsFPInit success\n");


The load() function is being called with nullptr when symbols are not loaded. However, the frontend panel initialization in FrontPanelConfig::getInstance() (line 97-100) was previously calling load() automatically, which has been removed. Now load() needs to be called explicitly with proper error handling. If the load fails or isn't called, the config won't be initialized, but m_isFPInitialized will still be set to true, leading to an inconsistent state.

Suggested change

_singleton.m_isFPInitialized = true;

INT_INFO("dsFPInit success\n");

// Attempt to load the front panel configuration

if (_singleton.load())

{

_singleton.m_isFPInitialized = true;

INT_INFO("dsFPInit and config load success\n");

}

else

{

INT_ERROR("dsFPInit succeeded but config load failed\n");

}

Moved the load() to Manager::Initialize().

Copilot · 2025-12-13T06:42:23Z

ds/audioOutputPortConfig.cpp

+            configuration.pKConfigs = kConfigs;
+            configSize = dsUTL_DIM(kConfigs);
+            configuration.pKConfigSize = &configSize;
+            configuration.pKPorts = kPorts;
+            portSize = dsUTL_DIM(kPorts);
+            configuration.pKPortSize = &portSize;


Similar to the issue in videoOutputPortConfig.cpp, the size pointers in the configuration structure point to local stack variables ('configSize', 'portSize'). These local variables will go out of scope when the function returns, making the pointers invalid. This could lead to undefined behavior if the configuration structure is accessed after function completion.

Copilot · 2025-12-13T06:42:23Z

ds/frontPanelConfig.cpp

+        configuration.pKFPDIndicatorColors = kIndicatorColors;
+        indicatorColorSize = dsUTL_DIM(kIndicatorColors);
+        configuration.pKFPDIndicatorColors_size = &indicatorColorSize;
+        configuration.pKIndicators = kIndicators;
+        indicatorSize = dsUTL_DIM(kIndicators);
+        configuration.pKIndicators_size = &indicatorSize;
+        configuration.pKTextDisplays = kTextDisplays;
+        textDisplaySize = dsUTL_DIM(kTextDisplays);
+        configuration.pKTextDisplays_size = &textDisplaySize;


Similar to the issue in other config files, the size pointers in the configuration structure point to local stack variables ('indicatorColorSize', 'indicatorSize', 'textDisplaySize'). These local variables will go out of scope when the function returns, making the pointers invalid. This could lead to undefined behavior if the configuration structure is accessed after function completion.

Copilot · 2025-12-13T06:42:24Z

ds/frontPanelConfig.cpp

+        int indicatorSize = (configuration->pKIndicators_size) ? *(configuration->pKIndicators_size) : -1;
+        int indicatorColorSize = (configuration->pKFPDIndicatorColors_size) ? *(configuration->pKFPDIndicatorColors_size) : -1;


The variable 'indicatorColorSize' is declared twice - once at line 355 with initialization to -1, and again at line 363 inside the if block where it's reassigned. The first declaration is shadowed by the second, making the initial assignment ineffective. Remove the second declaration at line 362 to avoid shadowing.

Suggested change

int indicatorSize = (configuration->pKIndicators_size) ? *(configuration->pKIndicators_size) : -1;

int indicatorColorSize = (configuration->pKFPDIndicatorColors_size) ? *(configuration->pKFPDIndicatorColors_size) : -1;

indicatorSize = (configuration->pKIndicators_size) ? *(configuration->pKIndicators_size) : -1;

indicatorColorSize = (configuration->pKFPDIndicatorColors_size) ? *(configuration->pKFPDIndicatorColors_size) : -1;

Copilot · 2025-12-13T06:42:24Z

ds/videoOutputPortConfig.cpp

+#define DEBUG 1 // Using for dumpconfig 
+


The 'DEBUG' macro is defined but never used in this file. If it's intended to control the dumpconfig functionality, it should be checked before calling dumpconfig(), or removed if unnecessary. Consider removing it to avoid confusion.

Suggested change

#define DEBUG 1 // Using for dumpconfig

apatel859 · 2025-12-15T16:18:25Z

ds/frontPanelConfig.cpp

+                fpdTextDisplayCfg->levels,
+                fpdTextDisplayCfg->maxHorizontalIterations,
+                fpdTextDisplayCfg->maxVerticalIterations,
+                (fpdTextDisplayCfg->supportedCharacters) ? fpdTextDisplayCfg->supportedCharacters : "NULL",


instead of NULL use macro that u have defined for "ABCDxxxx"

apatel859

fix it

apatel859 · 2025-12-15T16:21:32Z

ds/manager.cpp

-	device::VideoDeviceConfig::getInstance().load();
-	printf("%d:%s load completed\n", __LINE__, __FUNCTION__);
+    INT_INFO("Enter function");
+	loadDeviceCapabilities( device::DEVICE_CAPABILITY_VIDEO_PORT | 


why not DEVICE_CAPABILITY_FRONT_PANEL here

@apatel859 , This manager::load() called only from dsMgr context. And FrontPanel libds controlled from FrontPanel plugin. So we dont need FrontPanel config load here

Updated the DEVICE_CAPABILITY_FRONT_PANEL to use in dsmgr context.

yuvaramachandran-gurusamy · 2025-12-16T12:01:46Z

ds/audioOutputPortConfig.cpp

-	}
+void AudioOutputPortConfig::load(audioConfigs_t* dynamicAudioConfigs)
+{
+    static int configSize = -1, portSize = -1;


We dont need static variables here

yuvaramachandran-gurusamy · 2025-12-16T12:02:56Z

ds/frontPanelConfig.cpp

+    * 1. Create Supported Colors.
+    * 2. Create Indicators.
+    */
+    static int indicatorSize, indicatorColorSize, textDisplaySize;


We dont need static variables here

yuvaramachandran-gurusamy · 2025-12-16T12:03:18Z

ds/videoDeviceConfig.cpp

 #include "dslogger.h"
+#include "manager.hpp"

+#define DEBUG 1 // Using for dumpconfig 


Can we remove this

yuvaramachandran-gurusamy · 2025-12-16T12:03:28Z

ds/videoDeviceConfig.cpp

+
+void VideoDeviceConfig::load(videoDeviceConfig_t* dynamicVideoDeviceConfigs)
+{
+	static int configSize = -1;


We dont need static variables here

yuvaramachandran-gurusamy · 2025-12-16T12:03:54Z

ds/videoOutputPortConfig.cpp

 #include <string.h>
 using namespace std;

+#define DEBUG 1 // Using for dumpconfig 


Can we remove this

yuvaramachandran-gurusamy · 2025-12-16T12:07:10Z

ds/manager.cpp

+	loadDeviceCapabilities( device::DEVICE_CAPABILITY_VIDEO_PORT | 
+                            device::DEVICE_CAPABILITY_AUDIO_PORT |
+                            device::DEVICE_CAPABILITY_VIDEO_DEVICE |
+                            device::DEVICE_CAPABILITY_FRONT_PANEL);


@santoshcomcast , Can we remove FrontPanel here once @apatel859 agreed based on comments as above

yuvaramachandran-gurusamy · 2025-12-16T12:10:45Z

ds/include/frontPanelConfig.hpp

+    /* Terminate Front Panel */
+    void fPTerm();

+    void load(fpdConfigs_t* pDLHandle);


Can we rename pDLHandle into dynamicConfigPtr or something

yuvaramachandran-gurusamy · 2025-12-16T12:10:59Z

ds/audioOutputPortConfig.hpp

 	List<AudioOutputPortType> 	 getSupportedTypes();

-	void load();
+	void load(audioConfigs_t* pDLHandle);


Can we rename pDLHandle into dynamicConfigPtr or something

yuvaramachandran-gurusamy · 2025-12-16T12:11:09Z

ds/videoDeviceConfig.hpp

 	VideoDFC & getDefaultDFC();

-	void load();
+	void load(videoDeviceConfig_t* pDLHandle);


Can we rename pDLHandle into dynamicConfigPtr or something

yuvaramachandran-gurusamy · 2025-12-16T12:11:20Z

ds/videoOutputPortConfig.hpp

 	List<VideoResolution> getSupportedResolutions(bool isIgnoreEdid=false);

-	void load();
+	void load(videoPortConfigs_t* pDLHandle);


Can we rename pDLHandle into dynamicConfigPtr or something

Reason for change: consume the config variables using dlsym() in MW. Test Procedure: refer RDKEMW-8587 Risks: High Signed-off-by:gsanto722 <grandhi_santoshkumar@comcast.com>

Signed-off-by: grandhi_santoshkumar <grandhi_santoshkumar@comcast.com>

Reason for change: consume the config variables using dlsym() in MW. Test Procedure: refer RDKEMW-8587 Risks: High Signed-off-by:gsanto722 <grandhi_santoshkumar@comcast.com>

…logging format Signed-off-by: yuvaramachandran_gurusamy <yuvaramachandran_gurusamy@comcast.com>

Signed-off-by: yuvaramachandran_gurusamy <yuvaramachandran_gurusamy@comcast.com>

Reason for change: Address review comments. Test Procedure: refer RDKEMW-8587 Risks: High Signed-off-by:gsanto722 <grandhi_santoshkumar@comcast.com>

Reason for change: Addressed review comments. Test Procedure: refer RDKEMW-8587 Risks: High Signed-off-by:gsanto722 <grandhi_santoshkumar@comcast.com>

Reason for change: Address the other static config read places. Test Procedure: refer RDKEMW-8587 Risks: High Signed-off-by:gsanto722 <grandhi_santoshkumar@comcast.com>

Copilot

Pull request overview

Copilot reviewed 21 out of 21 changed files in this pull request and generated 23 comments.

Comments suppressed due to low confidence (1)

rpc/srv/dsDisplay.c:590

edidData is allocated via malloc() above, but the null-check is done on edid instead. This can lead to dereferencing a NULL allocation. Please change the check to if (edidData == NULL) and handle that case appropriately.

    dsDisplayEDID_t *edidData = (dsDisplayEDID_t*)malloc(sizeof(dsDisplayEDID_t));
    dsVideoPortType_t _VPortType = _GetDisplayPortType(handle);
    if (edid == NULL) {
        free(edidData);
    	return; // Handle malloc failure

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-12T10:55:01Z

ds/videoDeviceConfig.cpp

+        configuration.pKVideoDeviceConfigs = (dsVideoConfig_t *)kConfigs;
+		configSize = dsUTL_DIM(kConfigs);
+        configuration.pKVideoDeviceConfigs_size = &configSize;


This casts away const from kConfigs by assigning it to a non-const pointer type. Since these configs are static read-only data, the configuration struct member should be const dsVideoConfig_t* (and callers should treat it as const) to avoid accidental mutation / UB.

Copilot · 2026-02-12T10:55:01Z

ds/videoOutputPortConfig.cpp

+    std::lock_guard<std::mutex> lock(gSupportedResolutionsMutex);
+    *pResolutionCount = _supportedResolutions.size();
+
+    if (nullptr != pResolutions) {
+        // Reverse logic: Create pResolutions from _supportedResolutions


getVideoPortResolutions() writes _supportedResolutions.size() entries into pResolutions but the API doesn’t accept the caller’s buffer capacity. Callers passing a fixed-size buffer (as the RPC layer currently does) can be overflowed. Please extend the API to take a capacity (and clamp), or require/support a 2-pass pattern (first call with pResolutions=nullptr to get count, then allocate exactly that many).

Copilot · 2026-02-12T10:55:02Z

ds/videoOutputPortConfig.cpp

+            // Get default resolution name - pointer to persistent string in VideoResolution object
+            const VideoResolution& defaultRes = port.getDefaultResolution();
+            pPorts[i].defaultResolution = defaultRes.getName().c_str();
+        }


pPorts[i].defaultResolution is set to defaultRes.getName().c_str(), which points into a std::string owned by an object in _vPorts. If configs are ever reloaded (or _vPorts is modified), these pointers can dangle. Prefer copying into caller-owned storage or returning a stable string (e.g., store an owned std::string inside a wrapper struct rather than exposing raw pointers).

Copilot · 2026-02-12T10:55:02Z

ds/videoOutputPortConfig.cpp

+    *pPortCount = _vPorts.size();
+
+    if (nullptr != pPorts) {
+        // Reverse logic: Create pPorts from _vPorts
+        for (size_t i = 0; i < _vPorts.size(); i++) {


getVideoPortVPorts() writes _vPorts.size() entries into pPorts but the API doesn’t accept the caller’s buffer capacity, so callers using fixed arrays can be overflowed. Please add a capacity parameter (and clamp) or enforce a 2-pass size query + allocation pattern.

Copilot · 2026-02-12T10:55:02Z

ds/dslogger.cpp

+    char formatted[MAX_LOG_BUFF] = {'\0'};
+    enum LogLevel {INFO_LEVEL = 0, WARN_LEVEL, ERROR_LEVEL, DEBUG_LEVEL, TRACE_LEVEL};
+    const char *levelMap[] = { "INFO", "WARN", "ERROR", "DEBUG", "TRACE"};



ds_log() redefines enum LogLevel inside the function, which hides the public LogLevel type from dslogger.h and is confusing/unnecessary. Also, levelMap is indexed by priority without validating range; a bad value could read out of bounds. Please remove the inner enum and clamp/validate the priority before indexing levelMap.

Copilot · 2026-02-12T10:55:05Z

rpc/srv/dsAudio.c

+// Forward declaration for C++ function
+extern void dsGetAudioConfigs(int *pPortSize, dsAudioPortConfig_t *pkAudioPorts);


dsGetAudioConfigs is exported as extern "C" from ds/audioOutputPortConfig.cpp. Since this file is compiled as C++ (-x c++), this forward declaration should also be extern "C" to avoid unresolved symbols due to C++ name mangling.

Suggested change

// Forward declaration for C++ function

extern void dsGetAudioConfigs(int *pPortSize, dsAudioPortConfig_t *pkAudioPorts);

// Forward declaration for C++ function with C linkage

#ifdef __cplusplus

extern "C" {

#endif

extern void dsGetAudioConfigs(int *pPortSize, dsAudioPortConfig_t *pkAudioPorts);

#ifdef __cplusplus

}

#endif

Copilot · 2026-02-12T10:55:05Z

ds/videoDevice.cpp

+	// Get cached supported resolutions directly from _supportedResolutions
+	const std::vector<VideoResolution>& resolutions = VideoOutputPortConfig::getInstance()._supportedResolutions;


This directly reads VideoOutputPortConfig::_supportedResolutions without taking gSupportedResolutionsMutex, even though other code updates that vector under a lock. This introduces a potential data race and undefined behavior. Please add a thread-safe accessor on VideoOutputPortConfig that returns a copy of the names/resolutions under the mutex and use that here (instead of accessing the member directly).

Suggested change

// Get cached supported resolutions directly from _supportedResolutions

const std::vector<VideoResolution>& resolutions = VideoOutputPortConfig::getInstance()._supportedResolutions;

// Get cached supported resolutions via thread-safe accessor

std::vector<VideoResolution> resolutions = VideoOutputPortConfig::getInstance().getSupportedResolutions();

Copilot · 2026-02-12T10:55:06Z

rpc/srv/dsVideoPort.c

+    dsVideoPortPortConfig_t kVideoPortPorts[16] = {};
+
+    getVideoPortVPorts(&numPorts, kVideoPortPorts);


kVideoPortPorts is a fixed 16-element array, but getVideoPortVPorts() can report more than 16 ports via numPorts; the current code will read past the end of kVideoPortPorts in the later loop. Please clamp numPorts to the array capacity (or do a 2-pass call to fetch the required size and allocate accordingly).

Suggested change

dsVideoPortPortConfig_t kVideoPortPorts[16] = {};

getVideoPortVPorts(&numPorts, kVideoPortPorts);

dsVideoPortPortConfig_t kVideoPortPorts[16] = {};

const int maxPorts = (int)(sizeof(kVideoPortPorts) / sizeof(kVideoPortPorts[0]));

getVideoPortVPorts(&numPorts, kVideoPortPorts);

if (numPorts > maxPorts) {

INT_ERROR("[DsMgr] getVideoPortVPorts() reported %d ports, clamping to %d to avoid overflow\r\n",

numPorts, maxPorts);

numPorts = maxPorts;

} else if (numPorts < 0) {

INT_ERROR("[DsMgr] getVideoPortVPorts() reported invalid port count %d, treating as 0\r\n",

numPorts);

numPorts = 0;

}

Copilot · 2026-02-12T10:55:06Z

rpc/srv/dsVideoPort.c

+	dsVideoPortResolution_t kVidoeResolutionsSettings[16] = {};
+	int iCount = 0;
+	dsGetVideoPortResolutions(&iCount, kVidoeResolutionsSettings);
+


dsGetVideoPortResolutions() returns a count via iCount, but kVidoeResolutionsSettings is only 16 elements. If iCount > 16, dsGetVideoPortResolutions() will overflow this buffer. Consider calling once with pResolutions=nullptr to get the required count, then allocate accordingly (or clamp iCount to the provided capacity).

Copilot · 2026-02-12T10:55:06Z

rpc/srv/dsDisplay.c

+        // Print kVidoeResolutionsSettings array
+        INT_INFO("[DsMgr] ========== Dumping kVidoeResolutionsSettings Array ==========\r\n");
+        INT_INFO("[DsMgr] Total resolutions retrieved: %d\r\n", iCount);
+        for (int k = 0; k < iCount; k++) {
+            dsVideoPortResolution_t *res = &kVidoeResolutionsSettings[k];


This per-resolution dump is logged at INFO level inside filterEDIDResolution() and will run whenever EDID is processed; this can generate very large logs and affect performance. Please gate it behind a debug toggle (e.g., DS_LOG_LEVEL / /opt/dsMgrDumpDeviceConfigs) or remove after validation.

…) in MW. Reason for change: Implemented logic to load both dynamic and static configurations on the server side and removed unnecessary code. Test Procedure: refer RDKEMW-8587, RDKEMW-12193 Risks: High Signed-off-by:gsanto722 <grandhi_santoshkumar@comcast.com>

Copilot

Pull request overview

Copilot reviewed 30 out of 30 changed files in this pull request and generated 23 comments.

Comments suppressed due to low confidence (1)

rpc/srv/dsVideoPort.c:1926

Missing fallback handling: If the API signature mismatch for dsGetDefaultResolutionIndex() is not fixed and the function returns -1 (the initial value of g_defaultResIndex), or if resolutions is NULL, the function will return "720p" which may not be appropriate for all cases. Consider adding error logging or more robust fallback logic.

        	      int numResolutions = 0;
        	      dsVideoPortResolution_t* resolutions = NULL;
        	      int defaultIndex = 0;
        	      dsGetVideoPortResolutions(&numResolutions, &resolutions);
        	      dsGetDefaultResolutionIndex(&defaultIndex);
        	      if (resolutions && defaultIndex >= 0 && defaultIndex < numResolutions) {
        	          return resolution.assign(resolutions[defaultIndex].name);
        	      }
       		  }
         break;
      }
   }
   return resolution;

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-17T06:12:56Z

rpc/srv/dsAudioConfig.c

+        return -1;
+    }
+
+    memcpy(allocatedAudioPorts, source, size * sizeof(dsAudioPortConfig_t));


Potential shallow copy issue: The memcpy() performs a shallow copy of dsAudioPortConfig_t structures. If these structures contain pointer members, the pointers will be copied but not the data they point to. This could cause dangling pointer issues if the source data from dlsym() becomes invalid. Consider performing a deep copy of pointer members.

Copilot · 2026-02-17T06:12:56Z

rpc/srv/dsVideoPort.c

+	dsGetVideoPortResolutions(&iCount, &kVidoeResolutionsSettings);
+
+	// Print kVidoeResolutionsSettings array
+	INT_INFO("[DsMgr] ========== Dumping kVidoeResolutionsSettings Array in getPixelResolution ==========\r\n");
+	INT_INFO("[DsMgr] Total resolutions retrieved: %d\r\n", iCount);
+	for (int k = 0; k < iCount; k++) {
+		dsVideoPortResolution_t *res = &kVidoeResolutionsSettings[k];
+		INT_INFO("[DsMgr] [%d] name=%s, pixelRes=%d, aspectRatio=%d, stereoMode=%d, frameRate=%d, interlaced=%d\r\n",
+				 k, res->name, res->pixelResolution, res->aspectRatio, 
+				 res->stereoScopicMode, res->frameRate, res->interlaced);
+	}
+	INT_INFO("[DsMgr] ========== End of kVidoeResolutionsSettings Dump ==========\r\n");

-	for (unsigned int i = 0; i < dsUTL_DIM(kResolutions); i++)
+  	dsVideoPortResolution_t *Resn = &kVidoeResolutionsSettings[0]; 
+
+	for (int i = 0; i < iCount; i++)
 	{
-		Resn = &kResolutions[i];
+		Resn = &kVidoeResolutionsSettings[i];


Typo in variable name appears multiple times: "kVidoeResolutionsSettings" should be "kVideoResolutionsSettings" throughout this function (missing 'd' in "Video").

Copilot · 2026-02-17T06:12:56Z

rpc/srv/dsConfigs.c

+    if (NULL != pDLHandle) {
+        dlclose(pDLHandle);
+        pDLHandle = NULL;
+    }


Potential use-after-free: After dlclose() is called on line 152, the pointers obtained via dlsym() (stored in the dynamicAudioConfigs, dynamicVideoPortConfigs, and dynamicVideoDeviceConfigs structures) become invalid. These pointers are then passed to the load functions where they may be dereferenced through shallow copies. The dlclose() should be deferred until after all configuration data has been copied or the library should remain open for the lifetime of the process.

Suggested change

if (NULL != pDLHandle) {

dlclose(pDLHandle);

pDLHandle = NULL;

}

Copilot · 2026-02-17T06:12:57Z

ds/manager.cpp

+        dlclose(pDLHandle);
+        pDLHandle = nullptr;


Potential use-after-free: After dlclose() is called on line 180, the pointers obtained via dlsym() become invalid. These pointers are then used by the load functions. The dlclose() should be deferred until after all configuration data has been deep-copied, or the library should remain open for the lifetime of the process.

Suggested change

dlclose(pDLHandle);

pDLHandle = nullptr;

// Intentionally not calling dlclose(pDLHandle) here.

// Pointers obtained via dlsym() are used by the various load() functions,

// and closing the handle could invalidate those pointers and cause

// use-after-free issues. The library is kept open for the process lifetime.

Copilot · 2026-02-17T06:12:57Z

rpc/srv/dsAudioConfig.c

+void dsLoadAudioOutputPortConfig(const audioConfigs_t* dynamicAudioConfigs)
+{
+    int configSize = -1, portSize = -1;
+
+    INT_INFO("Using '%s' config", dynamicAudioConfigs ? "dynamic" : "static");
+
+    if (NULL != dynamicAudioConfigs)
+    {
+        // Reading Audio type configs
+        configSize = (dynamicAudioConfigs->pKConfigSize) ? *(dynamicAudioConfigs->pKConfigSize) : -1;
+        configSize = allocateAndCopyAudioConfigs(dynamicAudioConfigs->pKAudioConfigs, configSize, "dynamic");
+
+        // Reading Audio port configs
+        portSize = (dynamicAudioConfigs->pKPortSize) ? *(dynamicAudioConfigs->pKPortSize) : -1;
+        portSize = allocateAndCopyAudioPorts(dynamicAudioConfigs->pKAudioPorts, portSize, "dynamic");
+    }
+    else {
+        // Using static configuration
+        configSize = dsUTL_DIM(kConfigs);
+        configSize = allocateAndCopyAudioConfigs(kConfigs, configSize, "static");
+
+        portSize = dsUTL_DIM(kPorts);
+        portSize = allocateAndCopyAudioPorts(kPorts, portSize, "static");
+    }
+
+    // Store sizes for getter functions
+    g_audioConfigSize = configSize;
+    g_audioPortSize = portSize;
+
+    INT_INFO("Audio Config[%p] ConfigSize[%d] Ports[%p] PortSize[%d]",
+            audioConfiguration.pKAudioConfigs,
+            configSize,
+            audioConfiguration.pKAudioPorts,
+            portSize);
+    dumpconfig(&audioConfiguration);
+}


Potential memory leak on repeated calls: If dsLoadAudioOutputPortConfig() is called multiple times, the static pointers allocatedAudioConfigs and allocatedAudioPorts will be overwritten without freeing previously allocated memory. Either ensure this function is only called once, or free existing allocations before reallocating.

Copilot · 2026-02-17T06:13:00Z

rpc/srv/dsVideoDeviceConfig.c

+    if (nullptr == config) {
+        INT_ERROR("Video config is NULL");
+        return;
+    }
+    if ( -1 == access("/opt/dsMgrDumpDeviceConfigs", F_OK) ) {
+        INT_INFO("Dumping of Device configs is disabled");
+        return;
+    }
+
+    INT_INFO("\n=============== Starting to Dump VideoDevice Configs ===============\n");
+
+	if( nullptr != config->pKVideoDeviceConfigs )


Inconsistent use of NULL and nullptr. The file mixes NULL and nullptr for null pointer checks. In C files, use NULL consistently rather than nullptr which is a C++ keyword.

Copilot · 2026-02-17T06:13:00Z

rpc/srv/dsVideoPortConfig.c

+static dsVideoPortTypeConfig_t* allocatedVideoPortConfigs = NULL;
+static dsVideoPortPortConfig_t* allocatedVideoPortPorts = NULL;
+static dsVideoPortResolution_t* allocatedVideoPortResolutions = NULL;
+static int g_videoPortConfigSize = -1;
+static int g_videoPortPortSize = -1;
+static int g_videoPortResolutionSize = -1;
+static int g_defaultResIndex = -1;


Memory leak: allocated memory for allocatedVideoPortConfigs, allocatedVideoPortPorts, and allocatedVideoPortResolutions is never freed. There is no cleanup/release function provided, and the static pointers will retain these allocations for the lifetime of the process. Consider adding a cleanup function or documenting that these are intentionally persistent allocations.

Copilot · 2026-02-17T06:13:00Z

rpc/srv/dsAudioConfig.c

+static dsAudioPortConfig_t* allocatedAudioPorts = NULL;
+static int g_audioConfigSize = -1;
+static int g_audioPortSize = -1;
+


Memory leak: allocated memory for allocatedAudioConfigs and allocatedAudioPorts is never freed. While allocatedAudioPorts cleanup is attempted on line 127 only when subsequent allocation fails, there's no general cleanup function. Consider adding a cleanup function or documenting that these are intentionally persistent allocations.

Suggested change

static void freeAudioConfigurations(void)

{

if (allocatedAudioConfigs != NULL) {

free(allocatedAudioConfigs);

allocatedAudioConfigs = NULL;

g_audioConfigSize = -1;

}

if (allocatedAudioPorts != NULL) {

free(allocatedAudioPorts);

allocatedAudioPorts = NULL;

g_audioPortSize = -1;

}

}

Copilot · 2026-02-17T06:13:01Z

rpc/srv/dsVideoPortConfig.c

+void dsLoadVideoOutputPortConfig(const videoPortConfigs_t* dynamicVideoPortConfigs)
+{
+    int configSize = -1, portSize = -1, resolutionSize = -1;
+
+    INT_INFO("Using '%s' config", dynamicVideoPortConfigs ? "dynamic" : "static");
+
+    if (NULL != dynamicVideoPortConfigs)
+    {
+        // Reading Video Port Type configs
+        configSize = (dynamicVideoPortConfigs->pKVideoPortConfigs_size) ? *(dynamicVideoPortConfigs->pKVideoPortConfigs_size) : -1;
+        configSize = allocateAndCopyVideoPortConfigs(dynamicVideoPortConfigs->pKVideoPortConfigs, configSize, "dynamic");
+
+        // Reading Video Port Port configs
+        portSize = (dynamicVideoPortConfigs->pKVideoPortPorts_size) ? *(dynamicVideoPortConfigs->pKVideoPortPorts_size) : -1;
+        portSize = allocateAndCopyVideoPortPorts(dynamicVideoPortConfigs->pKVideoPortPorts, portSize, "dynamic");
+
+        // Reading Video Port Resolutions
+        resolutionSize = (dynamicVideoPortConfigs->pKResolutionsSettings_size) ? *(dynamicVideoPortConfigs->pKResolutionsSettings_size) : -1;
+        resolutionSize = allocateAndCopyVideoPortResolutions(dynamicVideoPortConfigs->pKVideoPortResolutionsSettings, resolutionSize, "dynamic");
+
+        // Reading Default Resolution Index
+        if (dynamicVideoPortConfigs->pKDefaultResIndex != NULL) {
+            g_defaultResIndex = *(dynamicVideoPortConfigs->pKDefaultResIndex);
+            videoPortConfiguration.pKDefaultResIndex = &g_defaultResIndex;
+            INT_INFO("Read default resolution index: %d (dynamic)", g_defaultResIndex);
+        } else {
+            INT_INFO("Default resolution index not available in dynamic config");
+        }
+    }
+    else {
+        // Using static configuration
+        configSize = dsUTL_DIM(kConfigs);
+        configSize = allocateAndCopyVideoPortConfigs(kConfigs, configSize, "static");
+
+        portSize = dsUTL_DIM(kPorts);
+        portSize = allocateAndCopyVideoPortPorts(kPorts, portSize, "static");
+
+        resolutionSize = dsUTL_DIM(kResolutions);
+        resolutionSize = allocateAndCopyVideoPortResolutions(kResolutions, resolutionSize, "static");
+
+        // Using static default resolution index
+        g_defaultResIndex = kDefaultResIndex;
+        videoPortConfiguration.pKDefaultResIndex = &g_defaultResIndex;
+        INT_INFO("Using static default resolution index: %d", g_defaultResIndex);
+    }
+
+    // Store sizes for getter functions
+    g_videoPortConfigSize = configSize;
+    g_videoPortPortSize = portSize;
+    g_videoPortResolutionSize = resolutionSize;
+
+    INT_INFO("VideoPort Config[%p] ConfigSize[%d] Ports[%p] PortSize[%d] Resolutions[%p] ResolutionSize[%d] DefaultResIndex[%d]",
+            videoPortConfiguration.pKVideoPortConfigs,
+            configSize,
+            videoPortConfiguration.pKVideoPortPorts,
+            portSize,
+            videoPortConfiguration.pKVideoPortResolutionsSettings,
+            resolutionSize,
+            g_defaultResIndex);
+    dumpconfig(&videoPortConfiguration);
+}


Potential memory leak on repeated calls: If dsLoadVideoOutputPortConfig() is called multiple times, the static pointers allocatedVideoPortConfigs, allocatedVideoPortPorts, and allocatedVideoPortResolutions will be overwritten without freeing previously allocated memory. Either ensure this function is only called once, or free existing allocations before reallocating.

Copilot · 2026-02-17T06:13:01Z

rpc/srv/dsAudioConfig.c

+        return -1;
+    }
+
+    memcpy(allocatedAudioConfigs, source, size * sizeof(dsAudioTypeConfig_t));


Potential shallow copy issue: The memcpy() performs a shallow copy of dsAudioTypeConfig_t structures. If these structures contain pointer members, the pointers will be copied but not the data they point to. This could cause dangling pointer issues if the source data from dlsym() becomes invalid. Consider performing a deep copy of nested structures and pointer members.

Copilot

Pull request overview

Copilot reviewed 30 out of 30 changed files in this pull request and generated 19 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-18T07:14:14Z

rpc/srv/dsAudioConfig.c

+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>


This file uses access()/F_OK but doesn’t include <unistd.h>, which can fail to compile in C++ (no implicit function declarations). Add the proper include.

Suggested change

#include <string.h>

#include <string.h>

#include <unistd.h>

Copilot · 2026-02-18T07:14:15Z

rpc/srv/dsVideoPort.c

+    const dsVideoPortPortConfig_t *kVideoPortPorts = NULL;
+
+    dsGetVideoPortPortConfigs(&numPorts, &kVideoPortPorts);
+


dsGetVideoPortPortConfigs() results aren’t validated before using kVideoPortPorts[i]. If configs aren’t loaded (or load failed), kVideoPortPorts can be NULL and numPorts can be 0/garbage, leading to crashes; add a NULL/size check and return a safe default/error.

Copilot · 2026-02-18T07:14:15Z

rpc/srv/dsVideoPortConfig.c

+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>


This file calls access()/F_OK but doesn’t include <unistd.h>. Add the proper include to ensure it compiles cleanly in C++ builds.

Suggested change

#include <string.h>

#include <string.h>

#include <unistd.h>

Copilot · 2026-02-18T07:14:15Z

rpc/srv/dsAudioConfig.c

+    // Store sizes for getter functions
+    *(audioConfiguration.pKConfigSize) = configSize;
+    *(audioConfiguration.pKPortSize) = portSize;


audioConfiguration.pKConfigSize / audioConfiguration.pKPortSize are never initialized but are dereferenced here. This will segfault; store sizes as plain int fields or allocate/assign backing storage before writing through these pointers.

Copilot · 2026-02-18T07:14:15Z

rpc/srv/dsMgr.c

+	INT_INFO("[%s]: Loading device configurations\r\n", __FUNCTION__);
+	dsLoadConfigs();


dsLoadConfigs() is now called during init, so the build must link in rpc/srv/dsConfigs.c (and the new *Config.c loaders it depends on). If these new sources aren’t added to the libdshalsrv sources list, this will fail at link time with an undefined reference.

Copilot · 2026-02-18T07:14:17Z

rpc/srv/dsVideoPortConfig.c

+        if (dynamicVideoPortConfigs->pKDefaultResIndex != NULL) {
+            *(videoPortConfiguration.pKDefaultResIndex) = *(dynamicVideoPortConfigs->pKDefaultResIndex);
+            INT_INFO("Read default resolution index: %d (dynamic)", *(videoPortConfiguration.pKDefaultResIndex));


videoPortConfiguration.pKDefaultResIndex is never initialized but is dereferenced when copying the default index. This will crash; store the default index as an int member or allocate/assign backing storage before dereferencing.

Copilot · 2026-02-18T07:14:18Z

rpc/srv/dsVideoPortConfig.c

+    *(videoPortConfiguration.pKVideoPortConfigs_size) = configSize;
+    *(videoPortConfiguration.pKVideoPortPorts_size) = portSize;
+    *(videoPortConfiguration.pKResolutionsSettings_size) = resolutionSize;


The *_size members in videoPortConfiguration are pointer fields but are never allocated/assigned; they’re dereferenced here, which will segfault. Store sizes as plain int members, or initialize these pointers to valid storage before writing through them.

Copilot · 2026-02-18T07:14:18Z

rpc/srv/dsConfigs.c

+    if (NULL != pDLHandle) {
+        dlclose(pDLHandle);
+        pDLHandle = NULL;
+    }


The config loaders shallow-copy structs that contain nested pointers (e.g., encoding arrays, supported resolutions, string pointers). After dlclose(), these pointers can become invalid and later use (getters/dumps) may crash. Keep the dlopen() handle alive (or use RTLD_NODELETE), or deep-copy nested data before closing.

Suggested change

if (NULL != pDLHandle) {

dlclose(pDLHandle);

pDLHandle = NULL;

}

Copilot · 2026-02-18T07:14:18Z

rpc/srv/dsVideoPort.c

+  	dsVideoPortResolution_t *Resn = &kVidoeResolutionsSettings[0]; 
+
+	for (int i = 0; i < iCount; i++)


kVidoeResolutionsSettings[0] is dereferenced without checking that dsGetVideoPortResolutions() returned a non-NULL pointer and iCount > 0. This can crash when configs aren’t loaded or the list is empty; add validation and a safe fallback.

Copilot · 2026-02-18T07:14:18Z

rpc/srv/dsAudio.c

-    numPorts = dsUTL_DIM(kSupportedPortTypes);
+    // Get audio port configurations from AudioOutputPortConfig
+    dsGetAudioPortConfigs(&numPorts, &kAudioPorts);



After dsGetAudioPortConfigs(&numPorts, &kAudioPorts), kAudioPorts is used without a NULL/size check. If config load failed or hasn’t run yet, this will crash; validate kAudioPorts != NULL and numPorts > 0 before indexing.

Suggested change

if ((kAudioPorts == NULL) || (numPorts <= 0))

{

INT_INFO("Error: Audio port configuration not available (numPorts=%d, kAudioPorts=%p)\r\n", numPorts, (void *)kAudioPorts);

return dsAUDIOPORT_TYPE_MAX;

}

Copilot

Pull request overview

Copilot reviewed 30 out of 30 changed files in this pull request and generated 29 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-23T16:10:30Z

rpc/srv/dsConfigs.c

+    INT_INFO("Entering capabilityType = 0x%08X\n", capabilityType);
+    dlerror(); /* clear old error */
+    pDLHandle = dlopen(RDK_DSHAL_NAME, RTLD_LAZY);
+    INT_INFO("DL Instance '%s'\n", (NULL == pDLHandle ? "NULL" : "Valid"));


Missing error handling for dlopen failure. If dlopen fails at line 91, pDLHandle will be NULL, but the code continues to process all capability types and only checks for NULL before calling LoadDLSymbols. Consider logging the dlerror() message and potentially returning early if the library cannot be opened, as all subsequent symbol lookups will fail.

Suggested change

INT_INFO("DL Instance '%s'\n", (NULL == pDLHandle ? "NULL" : "Valid"));

INT_INFO("DL Instance '%s'\n", (NULL == pDLHandle ? "NULL" : "Valid"));

if (NULL == pDLHandle) {

const char *pszError = dlerror();

INT_ERROR("Failed to open shared library '%s': %s\n",

RDK_DSHAL_NAME,

(NULL != pszError) ? pszError : "Unknown error");

}

Copilot · 2026-02-23T16:10:31Z

rpc/srv/dsVideoPortConfig.c

+    videoPortConfiguration.kDefaultResIndex = defaultResIndex;
+    INT_INFO("Store sizes videoPortConfiguration.kDefaultResIndex = %d\n", videoPortConfiguration.kDefaultResIndex);
+
+    INT_INFO("Store sizes configSize =%d, portSize =%d, resolutionSize = %d\n", configSize, portSize, resolutionSize);


Duplicate logging of sizes. Lines 252 and 256 both log the same information about configSize, portSize, and resolutionSize. Remove the duplicate logging statement.

Suggested change

INT_INFO("Store sizes configSize =%d, portSize =%d, resolutionSize = %d\n", configSize, portSize, resolutionSize);

Copilot · 2026-02-23T16:10:31Z

rpc/srv/dsDisplay.c

+                 k, port->id.type, port->id.index, 
+                 port->connectedAOP.type, port->connectedAOP.index, 
+                 port->defaultResolution);


Missing NULL check for port->defaultResolution when logging. Unlike line 1720 where there's a ternary check for NULL, line 678 directly uses port->defaultResolution without checking if it's NULL. Add a NULL check similar to other logging statements in the same function.

Suggested change

k, port->id.type, port->id.index,

port->connectedAOP.type, port->connectedAOP.index,

port->defaultResolution);

k, port->id.type, port->id.index,

port->connectedAOP.type, port->connectedAOP.index,

port->defaultResolution ? port->defaultResolution : "NULL");

Copilot · 2026-02-23T16:10:32Z

rpc/include/dsAudioConfig.h

+ * @param[out] outPorts Pointer to store the audio port configs array, or NULL
+ */
+void dsGetAudioPortConfigs(int* outPortSize, const dsAudioPortConfig_t** outPorts);
+


The function dsAudioConfigFree is called in dsMgr_term (line 149) but is not declared in the public header file dsAudioConfig.h. Add the function declaration to the header file for proper API visibility.

Suggested change

/**

* @brief Free any resources allocated for audio configurations

*

* This should be called during termination to release audio configuration

* resources initialized or loaded by the audio configuration APIs.

*/

void dsAudioConfigFree(void);

Copilot · 2026-02-23T16:10:32Z

ds/include/manager.hpp

+typedef enum DeviceCapabilityTypes {
+    DEVICE_CAPABILITY_INVALID = 0x00000000,
+    DEVICE_CAPABILITY_AUDIO_PORT = 0x00000001,
+    DEVICE_CAPABILITY_VIDEO_PORT = 0x00000002,
+    DEVICE_CAPABILITY_VIDEO_DEVICE = 0x00000004,
+    DEVICE_CAPABILITY_FRONT_PANEL = 0x00000008,
+    DEVICE_CAPABILITY_MAX = 0xFFFFFFFF
+}
+DeviceCapabilityTypes;
+
+typedef struct dlSymbolLookup {
+    const char* name;


Enum definition in header file should use scoped enum or namespace to avoid potential naming conflicts. The DeviceCapabilityTypes enum is defined in the header file without a C++ class scope, which could lead to naming conflicts with other code that defines similar constants. Consider using enum class instead of plain enum for better type safety.

Suggested change

typedef enum DeviceCapabilityTypes {

DEVICE_CAPABILITY_INVALID = 0x00000000,

DEVICE_CAPABILITY_AUDIO_PORT = 0x00000001,

DEVICE_CAPABILITY_VIDEO_PORT = 0x00000002,

DEVICE_CAPABILITY_VIDEO_DEVICE = 0x00000004,

DEVICE_CAPABILITY_FRONT_PANEL = 0x00000008,

DEVICE_CAPABILITY_MAX = 0xFFFFFFFF

}

DeviceCapabilityTypes;

typedef struct dlSymbolLookup {

const char* name;

enum class DeviceCapabilityTypes : unsigned int {

DEVICE_CAPABILITY_INVALID = 0x00000000,

DEVICE_CAPABILITY_AUDIO_PORT = 0x00000001,

DEVICE_CAPABILITY_VIDEO_PORT = 0x00000002,

DEVICE_CAPABILITY_VIDEO_DEVICE = 0x00000004,

DEVICE_CAPABILITY_FRONT_PANEL = 0x00000008,

DEVICE_CAPABILITY_MAX = 0xFFFFFFFF

};

typedef struct dlSymbolLookup {

const char* name;

const char* name;

Copilot · 2026-02-23T16:10:39Z

ds/audioOutputPortConfig.hpp

+typedef struct audioConfigs
+{
+	const dsAudioTypeConfig_t  *pKConfigs;
+	const dsAudioPortConfig_t  *pKPorts;
+	int *pKConfigSize;
+	int *pKPortSize;
+}audioConfigs_t;


Duplicate typedef definitions. The typedef audioConfigs_t is defined in multiple headers: dsAudioConfig.h, dsConfigs.h, and audioOutputPortConfig.hpp. This violates the One Definition Rule and could lead to compilation errors. Consider defining this type in a single common header and including that header wherever needed.

Copilot · 2026-02-23T16:10:40Z

rpc/srv/dsAudioConfig.c

+int dsLoadAudioOutputPortConfig(const audioConfigs_t* dynamicAudioConfigs)
+{
+    int configSize, portSize;
+    const dsAudioTypeConfig_t* audioConfigs;
+    const dsAudioPortConfig_t* audioPorts;
+    bool isDynamic;
+
+    INT_INFO("Using '%s' config\n", dynamicAudioConfigs ? "dynamic" : "static");
+
+    // Set up parameters based on config source
+    if (NULL != dynamicAudioConfigs) {
+        configSize = *(dynamicAudioConfigs->pKConfigSize);
+        portSize = *(dynamicAudioConfigs->pKPortSize);
+        audioConfigs = dynamicAudioConfigs->pKAudioConfigs;
+        audioPorts = dynamicAudioConfigs->pKAudioPorts;
+        isDynamic = true;
+    } else {
+        configSize = dsUTL_DIM(kConfigs);
+        portSize = dsUTL_DIM(kPorts);
+        audioConfigs = kConfigs;
+        audioPorts = kPorts;
+        isDynamic = false;
+    }
+
+    // Allocate and copy audio type configs
+    if (allocateAndCopyAudioConfigs(audioConfigs, configSize, isDynamic) == -1) {
+        INT_ERROR("Failed to allocate audio type configs\n");
+        return;
+    }
+
+    // Allocate and copy audio port configs
+    if (allocateAndCopyAudioPorts(audioPorts, portSize, isDynamic) == -1) {
+        INT_ERROR("Failed to allocate audio port configs\n");
+        // Clean up previously allocated memory
+        if (audioConfiguration.pKAudioConfigs != NULL) {
+            free(audioConfiguration.pKAudioConfigs);
+            audioConfiguration.pKAudioConfigs = NULL;
+        }
+        return;
+    }
+
+    INT_INFO("Store sizes configSize =%d, portSize =%d\n", configSize, portSize);
+
+    audioConfiguration.kConfigSize = configSize;
+    audioConfiguration.kPortSize = portSize;
+    INT_INFO("Store sizes audioConfiguration.kConfigSize = %d\n", audioConfiguration.kConfigSize);
+    INT_INFO("Store sizes audioConfiguration.kPortSize = %d\n", audioConfiguration.kPortSize);
+
+    INT_INFO("Audio Config[%p] ConfigSize[%d] Ports[%p] PortSize[%d]\n",
+            audioConfiguration.pKAudioConfigs ? audioConfiguration.pKAudioConfigs : NULL,
+            audioConfiguration.kConfigSize,
+            audioConfiguration.pKAudioPorts? audioConfiguration.pKAudioPorts : NULL,
+            audioConfiguration.kPortSize);
+
+    audioDumpconfig(&audioConfiguration);
+}


Function dsLoadAudioOutputPortConfig has inconsistent return type. The function signature indicates it returns int (line 138), but it uses 'return;' statements at lines 165 and 176, which would be appropriate for a void function. Either change the return type to void or ensure proper integer return values are used. Note that line 193 also lacks a return statement.

Copilot · 2026-02-23T16:10:40Z

rpc/include/dsVideoDeviceConfig.h

+ * 
+ * @param[in] dynamicVideoDeviceConfigs Pointer to dynamic video device configuration, or NULL for static config
+ */
+void dsLoadVideoDeviceConfig(const videoDeviceConfig_t* dynamicVideoDeviceConfigs);


Return type mismatch between declaration and implementation. The header file declares dsLoadVideoDeviceConfig as returning void (line 48), but the implementation in dsVideoDeviceConfig.c has it returning int (line 101). Update the implementation to match the header declaration by changing the return type to void.

Suggested change

void dsLoadVideoDeviceConfig(const videoDeviceConfig_t* dynamicVideoDeviceConfigs);

int dsLoadVideoDeviceConfig(const videoDeviceConfig_t* dynamicVideoDeviceConfigs);

Copilot · 2026-02-23T16:10:40Z

rpc/srv/dsVideoDeviceConfig.c

+        return -1;
+    }
+
+    memcpy(videoDeviceConfiguration.pKVideoDeviceConfigs, source, numElements * sizeof(dsVideoConfig_t));


Shallow copy of pointer data structure. The code performs memcpy of dsVideoConfig_t structures (line 96), which may contain pointers such as 'supportedDFCs'. If these pointers are allocated dynamically and the source is from a shared library that may be unloaded, this could lead to dangling pointers. Verify that the lifetime of the source data outlives the copied data or implement deep copy if necessary.

Copilot · 2026-02-23T16:10:40Z

rpc/srv/dsVideoPort.c

 static dsVideoResolution_t getPixelResolution(std::string &resolution )
 {
-  	dsVideoPortResolution_t *Resn = &kResolutions[kDefaultResIndex]; 
+	dsVideoPortResolution_t *kVidoeResolutionsSettings = NULL;


Typo in variable name. 'kVidoeResolutionsSettings' should be 'kVideoResolutionsSettings' (missing 'd' in 'Video'). This typo appears multiple times and should be corrected for consistency and readability.

Copilot

Pull request overview

Copilot reviewed 30 out of 30 changed files in this pull request and generated 15 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-23T17:00:46Z

rpc/include/dsVideoPortConfig.h

+ * 
+ * @param[in] dynamicVideoPortConfigs Pointer to dynamic video port configuration, or NULL for static config
+ */
+void dsLoadVideoOutputPortConfig(const videoPortConfigs_t* dynamicVideoPortConfigs);


Header/implementation mismatch: dsLoadVideoOutputPortConfig is declared void here, but the implementation returns int and callers assign its return value. This is a compile-time error in C++ and should be fixed by changing the declaration to return int (consistently everywhere).

Suggested change

void dsLoadVideoOutputPortConfig(const videoPortConfigs_t* dynamicVideoPortConfigs);

int dsLoadVideoOutputPortConfig(const videoPortConfigs_t* dynamicVideoPortConfigs);

Copilot · 2026-02-23T17:00:47Z

rpc/srv/dsConfigs.c

+    if (NULL != pDLHandle) {
+        dlclose(pDLHandle);
+        pDLHandle = NULL;
+    }


loadDeviceCapabilities() dlclose()s libdshal.so immediately after calling dsLoad*Config(), but the config loaders currently use memcpy() (shallow copy) of structs that contain pointers (e.g., names, supported arrays, defaultResolution). After dlclose(), those pointers may refer to unmapped memory, leading to use-after-free crashes. Fix by either keeping the dlopen handle alive for the lifetime of the copied configs, or deep-copying all pointed-to data before dlclose().

Suggested change

if (NULL != pDLHandle) {

dlclose(pDLHandle);

pDLHandle = NULL;

}

/* Keep libdshal.so loaded to ensure any pointers obtained from it remain valid. */

(void)pDLHandle;

Copilot · 2026-02-23T17:00:47Z

rpc/srv/dsVideoPort.c

-  	dsVideoPortResolution_t *Resn = &kResolutions[kDefaultResIndex]; 
+	dsVideoPortResolution_t *kVidoeResolutionsSettings = NULL;
+	int iCount = 0;
+	dsGetVideoPortResolutions(&iCount, &kVidoeResolutionsSettings);


getPixelResolution() assumes the resolutions array is non-NULL and non-empty. If dsGetVideoPortResolutions() returns iCount == 0 or a NULL pointer, later indexing (e.g., [0]) will crash. Add a guard and choose a safe fallback resolution when configs aren't available.

Suggested change

dsGetVideoPortResolutions(&iCount, &kVidoeResolutionsSettings);

dsGetVideoPortResolutions(&iCount, &kVidoeResolutionsSettings);

/* Guard against NULL or empty resolutions list to avoid invalid indexing. */

if ((kVidoeResolutionsSettings == NULL) || (iCount <= 0)) {

/* Fallback to a safe default pixel resolution (e.g., 720p). */

return dsVIDEO_PIXELRES_1280x720;

}

Copilot · 2026-02-23T17:00:47Z

ds/include/dslogger.h

 // conditionally enable debug logs, based on DS_LOG_LEVEL
 #if DS_LOG_LEVEL >= DEBUG_LEVEL
-#define INT_DEBUG(FORMAT, ...)      ds_log(DEBUG_LEVEL, fileName(__FILE__), __LINE__, FORMAT,  ##__VA_ARGS__ )
+#define INT_DEBUG(FORMAT, ...)      ds_log(DEBUG_LEVEL, fileName(__FILE__), __LINE__, __FUNCTION__, FORMAT,  ##__VA_ARGS__ )


This #if DS_LOG_LEVEL >= DEBUG_LEVEL check is evaluated by the preprocessor, but DEBUG_LEVEL is now an enum value (not a macro). As a result, the condition will typically be evaluated as 0 >= 0 and INT_DEBUG becomes enabled unexpectedly. Use numeric macros (or integer literals) for values used in preprocessor conditions.

Copilot · 2026-02-23T17:00:47Z

rpc/srv/dsMgr.c

+	INT_INFO("[%s]: Loading device configurations\r\n", __FUNCTION__);
+	dsLoadConfigs();
+    device::HostPersistence::getInstance().load();


dsMgr_init() now calls dsLoadConfigs(), which is implemented in the newly added rpc/srv/dsConfigs.c (and depends on the new *Config.c modules). The build files in rpc/srv still only list the old sources, so this will cause undefined reference / missing object issues unless the new sources are added to libdshalsrv_la_SOURCES and linked with libdl. Please update the build integration accordingly.

Copilot · 2026-02-23T17:00:49Z

rpc/srv/dsVideoPortConfig.c

+    // Allocate and copy video port type configs
+    if (allocateAndCopyVideoPortConfigs(videoPortConfigs, configSize, isDynamic) == -1) {
+        INT_ERROR("Failed to allocate video port configs\n");
+        return -1;
+    }


dsLoadVideoOutputPortConfig() overwrites the global videoPortConfiguration.* pointers with newly malloc'd buffers without first freeing any existing buffers. If configs are reloaded (e.g., re-init paths or repeated dsLoadConfigs()), this will leak memory and leave stale pointers. Consider calling dsVideoPortConfigFree() (or freeing existing fields) before allocating new buffers.

Copilot · 2026-02-23T17:00:49Z

rpc/srv/dsVideoPortConfig.c

+    if ( -1 == access("/opt/dsMgrDumpDeviceConfigs", F_OK) ) {
+        INT_INFO("Dumping of Device configs is disabled\n");


access() is used here but this file doesn't include <unistd.h>, which typically causes a missing declaration error when compiled as C++. Add <unistd.h> so access() / F_OK are declared.

Copilot · 2026-02-23T17:00:49Z

rpc/srv/dsAudioConfig.c

+    if ( -1 == access("/opt/dsMgrDumpDeviceConfigs", F_OK) ) {
+        INT_INFO("Dumping of Device configs is disabled\n");


access() is used here but this file doesn't include <unistd.h>, which is required for the access() / F_OK declarations under C++ compilation.

Copilot · 2026-02-23T17:00:50Z

ds/videoDeviceConfig.cpp

+#define DEBUG 1 // Using for dumpconfig 



#define DEBUG 1 is introduced but not used (and the comment suggests it's for dumpconfig). Consider removing it or wiring it into an actual conditional to avoid dead/debug-only macros in production builds.

Suggested change

#define DEBUG 1 // Using for dumpconfig

Copilot · 2026-02-23T17:00:50Z

rpc/srv/dsAudioConfig.c

+int dsLoadAudioOutputPortConfig(const audioConfigs_t* dynamicAudioConfigs)
+{
+    int configSize, portSize;
+    const dsAudioTypeConfig_t* audioConfigs;
+    const dsAudioPortConfig_t* audioPorts;


dsLoadAudioOutputPortConfig() writes new malloc'd buffers into the global audioConfiguration but does not free any previously allocated buffers first. If configs can be loaded more than once during process lifetime, this will leak memory; consider freeing existing buffers at the start of the load (or calling dsAudioConfigFree() before reloading).

yuvaramachandran-gurusamy requested a review from a team as a code owner December 11, 2025 14:56

rdkcmf-jenkins reviewed Dec 12, 2025

View reviewed changes

yuvaramachandran-gurusamy requested a review from rdkcmf-jenkins December 12, 2025 22:59

apatel859 reviewed Dec 13, 2025

View reviewed changes

apatel859 requested changes Dec 13, 2025

View reviewed changes

apatel859 requested a review from Copilot December 13, 2025 06:35

Copilot started reviewing on behalf of apatel859 December 13, 2025 06:36 View session

Copilot AI reviewed Dec 13, 2025

View reviewed changes

apatel859 reviewed Dec 15, 2025

View reviewed changes

apatel859 requested changes Dec 15, 2025

View reviewed changes

apatel859 reviewed Dec 15, 2025

View reviewed changes

yuvaramachandran-gurusamy commented Dec 16, 2025

View reviewed changes

santoshcomcast added 16 commits January 6, 2026 10:48

RDKEMW-8587: consume the config variables using dlsym() in MW.

6abd424

Reason for change: consume the config variables using dlsym() in MW. Test Procedure: refer RDKEMW-8587 Risks: High Signed-off-by:gsanto722 <grandhi_santoshkumar@comcast.com>

fix build issue

59bbd5c

fix build issue

b08a24d

Signed-off-by: grandhi_santoshkumar <grandhi_santoshkumar@comcast.com>

addressed review comments

9cc6b5f

fix build issue

ade5a61

fix build issue

6ac6e50

fix build issue

2dc944b

fix build issue

f6f3d48

fix build issue

441361b

fix coverity issue and removed unwanted code.

8d36de9

Fix build issue.

c676229

Addressed review comments.

968901d

Addressed review comments.

513b3c7

addressed review comments.

fb4785d

RDKEMW-8587: consume the config variables using dlsym() in MW.

35661b4

Reason for change: consume the config variables using dlsym() in MW. Test Procedure: refer RDKEMW-8587 Risks: High Signed-off-by:gsanto722 <grandhi_santoshkumar@comcast.com>

RDKEMW-8587: consume the config variables using dlsym() in MW.

4c52eeb

Reason for change: consume the config variables using dlsym() in MW. Test Procedure: refer RDKEMW-8587 Risks: High Signed-off-by:gsanto722 <grandhi_santoshkumar@comcast.com>

yuvaramachandran-gurusamy and others added 4 commits January 6, 2026 10:48

RDKEMW-8587: Fix FPD std::logical_error crash issue and enhanced the …

cca3783

…logging format Signed-off-by: yuvaramachandran_gurusamy <yuvaramachandran_gurusamy@comcast.com>

RDKEMW-8587: Fixed the coverity issues

01776ad

Signed-off-by: yuvaramachandran_gurusamy <yuvaramachandran_gurusamy@comcast.com>

RDKEMW-8587: consume the config variables using dlsym() in MW.

17564c6

Reason for change: Address review comments. Test Procedure: refer RDKEMW-8587 Risks: High Signed-off-by:gsanto722 <grandhi_santoshkumar@comcast.com>

RDKEMW-8587: consume the config variables using dlsym() in MW.

b8cdf99

Reason for change: Addressed review comments. Test Procedure: refer RDKEMW-8587 Risks: High Signed-off-by:gsanto722 <grandhi_santoshkumar@comcast.com>

santoshcomcast force-pushed the feature/RDKEMW-8587-dlsym branch from ef2294f to b8cdf99 Compare January 6, 2026 05:18

RDKEMW-8587: consume the config variables using dlsym() in MW.

664fd4f

Reason for change: Address the other static config read places. Test Procedure: refer RDKEMW-8587 Risks: High Signed-off-by:gsanto722 <grandhi_santoshkumar@comcast.com>

Copilot AI review requested due to automatic review settings February 12, 2026 10:45

Copilot started reviewing on behalf of santoshcomcast February 12, 2026 10:45 View session

Copilot AI reviewed Feb 12, 2026

View reviewed changes

santoshcomcast added 2 commits February 16, 2026 15:32

Added missing dsConfigs.c file

e3b7b2b

Copilot AI review requested due to automatic review settings February 17, 2026 06:05

Copilot started reviewing on behalf of santoshcomcast February 17, 2026 06:05 View session

Copilot AI reviewed Feb 17, 2026

View reviewed changes

santoshcomcast added 2 commits February 17, 2026 12:55

addressed internal review comments and cleanup code.

f8c568e

fix build linking issue

901a227

Copilot AI review requested due to automatic review settings February 18, 2026 07:04

Copilot started reviewing on behalf of santoshcomcast February 18, 2026 07:04 View session

Copilot AI reviewed Feb 18, 2026

View reviewed changes

santoshcomcast added 2 commits February 19, 2026 12:28

Fix run time errors.

ef6597a

Add free() funcation while dsTerm and addressing review comments.

2a0902b

Copilot AI review requested due to automatic review settings February 23, 2026 16:00

Copilot started reviewing on behalf of santoshcomcast February 23, 2026 16:01 View session

Copilot AI reviewed Feb 23, 2026

View reviewed changes

santoshcomcast added 2 commits February 23, 2026 16:15

Add missing headrfile changes

b4a0fba

Fix build errors.

31735bd

Copilot AI review requested due to automatic review settings February 23, 2026 16:51

Copilot started reviewing on behalf of santoshcomcast February 23, 2026 16:51 View session

Copilot AI reviewed Feb 23, 2026

View reviewed changes

fix build issue

f2369dd

	static int IsInitialized; //!< Indicates the application has initialized with devicettings modules.
	static int IsInitialized; //!< Indicates the application has initialized with Device Settings modules.

	INT_INFO("Entering ... count %d with thread id %lu\n",IsInitialized,pthread_self());
	INT_INFO("Entering ... count %d with thread id %lu",IsInitialized,pthread_self());

	INT_ERROR("No valid text display configuration found\n");
	INT_ERROR("No valid text display configuration found");

		if (( false == m_isFPInitialized) \|\| (true == m_isFPConfigLoaded)) {
		INT_ERROR("'%s'", (!m_isFPInitialized) ? "Front Panel not initialized" : "Front Panel Config already loaded");

-    if (( false == m_isFPInitialized) || (true == m_isFPConfigLoaded)) {
-        INT_ERROR("'%s'", (!m_isFPInitialized) ? "Front Panel not initialized" : "Front Panel Config already loaded");
+    if (!m_isFPInitialized) {
+        INT_ERROR("'Front Panel not initialized'");
+        throw IllegalArgumentException("Front Panel not initialized");
+    }
+    if (m_isFPConfigLoaded) {
+        INT_ERROR("'Front Panel Config already loaded'");

		_singleton.m_isFPInitialized = true;
		INT_INFO("dsFPInit success\n");

-                _singleton.m_isFPInitialized = true;
-                INT_INFO("dsFPInit success\n");
+                // Attempt to load the front panel configuration
+                if (_singleton.load())
+                {
+                    _singleton.m_isFPInitialized = true;
+                    INT_INFO("dsFPInit and config load success\n");
+                }
+                else
+                {
+                    INT_ERROR("dsFPInit succeeded but config load failed\n");
+                }

		int indicatorSize = (configuration->pKIndicators_size) ? *(configuration->pKIndicators_size) : -1;
		int indicatorColorSize = (configuration->pKFPDIndicatorColors_size) ? *(configuration->pKFPDIndicatorColors_size) : -1;

		// Forward declaration for C++ function
		extern void dsGetAudioConfigs(int pPortSize, dsAudioPortConfig_t pkAudioPorts);

-// Forward declaration for C++ function
-extern void dsGetAudioConfigs(int *pPortSize, dsAudioPortConfig_t *pkAudioPorts);
+// Forward declaration for C++ function with C linkage
+#ifdef __cplusplus
+extern "C" {
+#endif
+extern void dsGetAudioConfigs(int *pPortSize, dsAudioPortConfig_t *pkAudioPorts);
+#ifdef __cplusplus
+}
+#endif

		// Get cached supported resolutions directly from _supportedResolutions
		const std::vector<VideoResolution>& resolutions = VideoOutputPortConfig::getInstance()._supportedResolutions;

		dsVideoPortPortConfig_t kVideoPortPorts[16] = {};

		getVideoPortVPorts(&numPorts, kVideoPortPorts);

-    dsVideoPortPortConfig_t kVideoPortPorts[16] = {};
-    getVideoPortVPorts(&numPorts, kVideoPortPorts);
+    dsVideoPortPortConfig_t kVideoPortPorts[16] = {};
+    const int maxPorts = (int)(sizeof(kVideoPortPorts) / sizeof(kVideoPortPorts[0]));
+    getVideoPortVPorts(&numPorts, kVideoPortPorts);
+    if (numPorts > maxPorts) {
+        INT_ERROR("[DsMgr] getVideoPortVPorts() reported %d ports, clamping to %d to avoid overflow\r\n",
+                  numPorts, maxPorts);
+        numPorts = maxPorts;
+    } else if (numPorts < 0) {
+        INT_ERROR("[DsMgr] getVideoPortVPorts() reported invalid port count %d, treating as 0\r\n",
+                  numPorts);
+        numPorts = 0;
+    }

-        dlclose(pDLHandle);
-        pDLHandle = nullptr;
+        // Intentionally not calling dlclose(pDLHandle) here.
+        // Pointers obtained via dlsym() are used by the various load() functions,
+        // and closing the handle could invalidate those pointers and cause
+        // use-after-free issues. The library is kept open for the process lifetime.

+static void freeAudioConfigurations(void)
+{
+    if (allocatedAudioConfigs != NULL) {
+        free(allocatedAudioConfigs);
+        allocatedAudioConfigs = NULL;
+        g_audioConfigSize = -1;
+    }
+    if (allocatedAudioPorts != NULL) {
+        free(allocatedAudioPorts);
+        allocatedAudioPorts = NULL;
+        g_audioPortSize = -1;
+    }
+}

		const dsVideoPortPortConfig_t *kVideoPortPorts = NULL;

		dsGetVideoPortPortConfigs(&numPorts, &kVideoPortPorts);

		INT_INFO("[%s]: Loading device configurations\r\n", __FUNCTION__);
		dsLoadConfigs();

		dsVideoPortResolution_t *Resn = &kVidoeResolutionsSettings[0];

		for (int i = 0; i < iCount; i++)

+    if ((kAudioPorts == NULL) || (numPorts <= 0))
+    {
+        INT_INFO("Error: Audio port configuration not available (numPorts=%d, kAudioPorts=%p)\r\n", numPorts, (void *)kAudioPorts);
+        return dsAUDIOPORT_TYPE_MAX;
+    }

-    INT_INFO("DL Instance '%s'\n", (NULL == pDLHandle ? "NULL" : "Valid"));
+    INT_INFO("DL Instance '%s'\n", (NULL == pDLHandle ? "NULL" : "Valid"));
+    if (NULL == pDLHandle) {
+        const char *pszError = dlerror();
+        INT_ERROR("Failed to open shared library '%s': %s\n",
+                  RDK_DSHAL_NAME,
+                  (NULL != pszError) ? pszError : "Unknown error");
+    }

Comments

Conversation

yuvaramachandran-gurusamy commented Dec 11, 2025 • edited by santoshcomcast Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

apatel859 left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 13, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 13, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

apatel859 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yuvaramachandran-gurusamy commented Dec 11, 2025 •

edited by santoshcomcast

Loading

+/**
+ * @brief Free any resources allocated for audio configurations
+ *
+ * This should be called during termination to release audio configuration
+ * resources initialized or loaded by the audio configuration APIs.
+ */
+void dsAudioConfigFree(void);

	void dsLoadVideoDeviceConfig(const videoDeviceConfig_t* dynamicVideoDeviceConfigs);
	int dsLoadVideoDeviceConfig(const videoDeviceConfig_t* dynamicVideoDeviceConfigs);

	void dsLoadVideoOutputPortConfig(const videoPortConfigs_t* dynamicVideoPortConfigs);
	int dsLoadVideoOutputPortConfig(const videoPortConfigs_t* dynamicVideoPortConfigs);

-	dsGetVideoPortResolutions(&iCount, &kVidoeResolutionsSettings);
+	dsGetVideoPortResolutions(&iCount, &kVidoeResolutionsSettings);
+	/* Guard against NULL or empty resolutions list to avoid invalid indexing. */
+	if ((kVidoeResolutionsSettings == NULL) || (iCount <= 0)) {
+		/* Fallback to a safe default pixel resolution (e.g., 720p). */
+		return dsVIDEO_PIXELRES_1280x720;
+	}

		if ( -1 == access("/opt/dsMgrDumpDeviceConfigs", F_OK) ) {
		INT_INFO("Dumping of Device configs is disabled\n");