RDKCOM-5335: RDKBDEV-3195 update AnscEqualString() -> strcmp() #419

Sukanya673 · 2025-06-23T09:52:11Z

Reason for change:
The libc strcmp() function is likely to be much more optimized than the legacy AnscEqualString(). Test Procedure: Sanity
Risks: None.
Signed-off-by: Andre McCurdy [email protected]
(cherry picked from commit 8874e6e)

Reason for change: The libc strcmp() function is likely to be much more optimized than the legacy AnscEqualString(). Test Procedure: Sanity Risks: None. Signed-off-by: Andre McCurdy <[email protected]> (cherry picked from commit 8874e6e)

narendradandu · 2025-07-01T23:22:03Z

please resolve conflicts, AnscEqualString handles NULL cases, can you check and confirm if all the inputs have null check validations before calling strcmp

CLAassistant · 2025-08-14T15:40:11Z

All committers have signed the CLA.

Pavlo-Uvarov · 2025-09-22T16:37:08Z

source/dml/tr_181/sbapi/cosa_wifi_apis.c

+	    {
+	        CcspTraceInfo(("%s", LogMsg));
+	    }
+	}


Suggested change

}

}

else

{

CcspTraceInfo(("%s", LogMsg));

}

Pavlo-Uvarov · 2025-09-22T16:39:13Z

source/dml/tr_181/sbapi/cosa_wifi_apis.c

+		if(wifiFreqBandMap[seqCounter].wifiFreqBandStr != NULL) {
+	        if (strcmp(pFreqBandStr, wifiFreqBandMap[seqCounter].wifiFreqBandStr) == 0)
+	        {
+	            *pFreqBandEnum = wifiFreqBandMap[seqCounter].halWifiFreqBand;
+	            ccspWifiDbgPrint(CCSP_WIFI_TRACE, "%s input : %s OperatingFrequencyBand : %d\n", __FUNCTION__, pFreqBandStr, *pFreqBandEnum);
+	            isBandInvalid = FALSE;
+	            break;
+	        }
+		}


Please remove redundant nesting:

Suggested change

if(wifiFreqBandMap[seqCounter].wifiFreqBandStr != NULL) {

if (strcmp(pFreqBandStr, wifiFreqBandMap[seqCounter].wifiFreqBandStr) == 0)

{

*pFreqBandEnum = wifiFreqBandMap[seqCounter].halWifiFreqBand;

ccspWifiDbgPrint(CCSP_WIFI_TRACE, "%s input : %s OperatingFrequencyBand : %d\n", __FUNCTION__, pFreqBandStr, *pFreqBandEnum);

isBandInvalid = FALSE;

break;

}

}

if((wifiFreqBandMap[seqCounter].wifiFreqBandStr != NULL) && (strcmp(pFreqBandStr, wifiFreqBandMap[seqCounter].wifiFreqBandStr) == 0))

{

*pFreqBandEnum = wifiFreqBandMap[seqCounter].halWifiFreqBand;

ccspWifiDbgPrint(CCSP_WIFI_TRACE, "%s input : %s OperatingFrequencyBand : %d\n", __FUNCTION__, pFreqBandStr, *pFreqBandEnum);

isBandInvalid = FALSE;

break;

}

Pavlo-Uvarov · 2025-09-22T16:42:52Z

source/dml/tr_181/sbapi/cosa_wifi_apis.c

+		if((environment != NULL) && (wifiEnviromentMap[seqCounter].environment != NULL)) {
+	        if (strcmp(environment, wifiEnviromentMap[seqCounter].environment) == 0)
+	        {
+	            *operatingEnvironment = wifiEnviromentMap[seqCounter].operatingEnvironment;
+	            ccspWifiDbgPrint(CCSP_WIFI_TRACE, "%s input : %s OperatingEnvironment : %d\n", __FUNCTION__, pRegDomain, *operatingEnvironment);
+	            isregDomainInvalid = FALSE;
+	            break;
+	        }
+		}


Please remove redundant nesting:

Suggested change

if((environment != NULL) && (wifiEnviromentMap[seqCounter].environment != NULL)) {

if (strcmp(environment, wifiEnviromentMap[seqCounter].environment) == 0)

{

*operatingEnvironment = wifiEnviromentMap[seqCounter].operatingEnvironment;

ccspWifiDbgPrint(CCSP_WIFI_TRACE, "%s input : %s OperatingEnvironment : %d\n", __FUNCTION__, pRegDomain, *operatingEnvironment);

isregDomainInvalid = FALSE;

break;

}

}

if (((environment != NULL) && (wifiEnviromentMap[seqCounter].environment != NULL)) && (strcmp(environment, wifiEnviromentMap[seqCounter].environment) == 0))

{

*operatingEnvironment = wifiEnviromentMap[seqCounter].operatingEnvironment;

ccspWifiDbgPrint(CCSP_WIFI_TRACE, "%s input : %s OperatingEnvironment : %d\n", __FUNCTION__, pRegDomain, *operatingEnvironment);

isregDomainInvalid = FALSE;

break;

}

Pavlo-Uvarov · 2025-09-22T16:44:03Z

source/dml/tr_181/sbapi/cosa_wifi_apis.c

+		if((tmp_regDomain != NULL) && (wifiCountryMapMembers[seqCounter].countryStr != NULL)) {
+	        if (strcmp(tmp_regDomain, wifiCountryMapMembers[seqCounter].countryStr) == 0)
+	        {
+	            *countryCode = wifiCountryMapMembers[seqCounter].countryCode;
+	            ccspWifiDbgPrint(CCSP_WIFI_TRACE, "%s input : %s Countrycode : %d\n", __FUNCTION__, pRegDomain, *countryCode);
+	            isregDomainInvalid = FALSE;
+	            break;
+	        }
+		}


Please remove redundant nesting:

Suggested change

if((tmp_regDomain != NULL) && (wifiCountryMapMembers[seqCounter].countryStr != NULL)) {

if (strcmp(tmp_regDomain, wifiCountryMapMembers[seqCounter].countryStr) == 0)

{

*countryCode = wifiCountryMapMembers[seqCounter].countryCode;

ccspWifiDbgPrint(CCSP_WIFI_TRACE, "%s input : %s Countrycode : %d\n", __FUNCTION__, pRegDomain, *countryCode);

isregDomainInvalid = FALSE;

break;

}

}

if (((tmp_regDomain != NULL) && (wifiCountryMapMembers[seqCounter].countryStr != NULL)) && (strcmp(tmp_regDomain, wifiCountryMapMembers[seqCounter].countryStr) == 0))

{

*countryCode = wifiCountryMapMembers[seqCounter].countryCode;

ccspWifiDbgPrint(CCSP_WIFI_TRACE, "%s input : %s Countrycode : %d\n", __FUNCTION__, pRegDomain, *countryCode);

isregDomainInvalid = FALSE;

break;

}

Pavlo-Uvarov · 2025-09-22T16:45:16Z

source/dml/tr_181/sbapi/cosa_wifi_apis.c

+			if(wifiDataTxRateMap[seqCounter].DataTxRateStr != NULL) {
+	            if (strcmp(token, wifiDataTxRateMap[seqCounter].DataTxRateStr) == 0)
+	            {
+	                isRateInvalid = FALSE;
+	            }
+			}


Please remove redundant nesting:

Suggested change

if(wifiDataTxRateMap[seqCounter].DataTxRateStr != NULL) {

if (strcmp(token, wifiDataTxRateMap[seqCounter].DataTxRateStr) == 0)

{

isRateInvalid = FALSE;

}

}

if ((wifiDataTxRateMap[seqCounter].DataTxRateStr != NULL) && (strcmp(token, wifiDataTxRateMap[seqCounter].DataTxRateStr) == 0))

{

isRateInvalid = FALSE;

}

Pavlo-Uvarov · 2025-09-22T19:03:49Z

source/dml/tr_181/ml/cosa_wifi_dml.c

+	                //WiFi_SetHS2Status(vap_pcfg->vap_index, false, true);
+		*pBool = interworking_pcfg->passpoint.enable;


Align:

Suggested change

//WiFi_SetHS2Status(vap_pcfg->vap_index, false, true);

*pBool = interworking_pcfg->passpoint.enable;

//WiFi_SetHS2Status(vap_pcfg->vap_index, false, true);

*pBool = interworking_pcfg->passpoint.enable;

Pavlo-Uvarov · 2025-09-22T19:06:06Z

source/dml/tr_181/ml/cosa_wifi_dml.c

+	if(ParamName != NULL) {
+	    if (strcmp(ParamName, "Enable") == 0) {
+	        if(bValue == vapInfo->u.bss_info.interworking.passpoint.enable){
+	            CcspTraceWarning(("Passpoint value Already configured. Return Success\n"));
+	            return TRUE;
+	        }
+
+	        retPsmGet = PSM_Get_Record_Value2(bus_handle,g_Subsystem, "Device.DeviceInfo.X_RDKCENTRAL-COM_RFC.Feature.WiFi-Passpoint.Enable", NULL, &strValue);
+	        if ((retPsmGet != CCSP_SUCCESS) || (false == _ansc_atoi(strValue)) || (FALSE == _ansc_atoi(strValue))){
+	            ((CCSP_MESSAGE_BUS_INFO *)bus_handle)->freefunc(strValue);
+	            CcspTraceWarning(("Cannot Enable Passpoint. RFC Disabled\n"));
+	            return FALSE;
+	        }
+
+	        if(false == vapInfo->u.bss_info.interworking.interworking.interworkingEnabled){
+	            CcspTraceWarning(("Cannot Enable Passpoint. Interworking Disabled\n"));
+	            return FALSE;
+	        }
+		    vapInfo->u.bss_info.interworking.passpoint.enable = bValue;
+		    set_dml_cache_vap_config_changed(instance_number - 1);
+		    return TRUE;
+	    }
+	}


Please remove redundant nesting:

Suggested change

if(ParamName != NULL) {

if (strcmp(ParamName, "Enable") == 0) {

if(bValue == vapInfo->u.bss_info.interworking.passpoint.enable){

CcspTraceWarning(("Passpoint value Already configured. Return Success\n"));

return TRUE;

}

retPsmGet = PSM_Get_Record_Value2(bus_handle,g_Subsystem, "Device.DeviceInfo.X_RDKCENTRAL-COM_RFC.Feature.WiFi-Passpoint.Enable", NULL, &strValue);

if ((retPsmGet != CCSP_SUCCESS) || (false == _ansc_atoi(strValue)) || (FALSE == _ansc_atoi(strValue))){

((CCSP_MESSAGE_BUS_INFO *)bus_handle)->freefunc(strValue);

CcspTraceWarning(("Cannot Enable Passpoint. RFC Disabled\n"));

return FALSE;

}

if(false == vapInfo->u.bss_info.interworking.interworking.interworkingEnabled){

CcspTraceWarning(("Cannot Enable Passpoint. Interworking Disabled\n"));

return FALSE;

}

vapInfo->u.bss_info.interworking.passpoint.enable = bValue;

set_dml_cache_vap_config_changed(instance_number - 1);

return TRUE;

}

}

if ((ParamName != NULL) && (strcmp(ParamName, "Enable") == 0))

{

if(bValue == vapInfo->u.bss_info.interworking.passpoint.enable){

CcspTraceWarning(("Passpoint value Already configured. Return Success\n"));

return TRUE;

}

retPsmGet = PSM_Get_Record_Value2(bus_handle,g_Subsystem, "Device.DeviceInfo.X_RDKCENTRAL-COM_RFC.Feature.WiFi-Passpoint.Enable", NULL, &strValue);

if ((retPsmGet != CCSP_SUCCESS) || (false == _ansc_atoi(strValue)) || (FALSE == _ansc_atoi(strValue))){

((CCSP_MESSAGE_BUS_INFO *)bus_handle)->freefunc(strValue);

CcspTraceWarning(("Cannot Enable Passpoint. RFC Disabled\n"));

return FALSE;

}

if(false == vapInfo->u.bss_info.interworking.interworking.interworkingEnabled){

CcspTraceWarning(("Cannot Enable Passpoint. Interworking Disabled\n"));

return FALSE;

}

vapInfo->u.bss_info.interworking.passpoint.enable = bValue;

set_dml_cache_vap_config_changed(instance_number - 1);

return TRUE;

}

Pavlo-Uvarov · 2025-09-22T19:07:29Z

source/dml/tr_181/ml/cosa_wifi_dml.c

+	if(Parameters != NULL) {
+	    if (strcmp(ParamName, "Parameters") == 0)
+	    {
+	        if (strcmp((char*)vapInfo->u.bss_info.interworking.passpoint.hs2Parameters, pString) == 0){
+	            return TRUE;
+	        }else {
+	             cJSON *p_root = cJSON_Parse(pString);
+	            if(p_root == NULL) {
+	                wifi_util_dbg_print(WIFI_DMCLI,"%s:%d Invalid json for vap %s\n", __FUNCTION__,__LINE__,pcfg->vap_name);
+	                return FALSE;
+	            }
+	            AnscCopyString((char*)vapInfo->u.bss_info.interworking.passpoint.hs2Parameters,pString);
+		    set_dml_cache_vap_config_changed(instance_number - 1);
+	            return TRUE;
+	        }
+	    }
+	}


Please remove redundant nesting:

Suggested change

if(Parameters != NULL) {

if (strcmp(ParamName, "Parameters") == 0)

{

if (strcmp((char*)vapInfo->u.bss_info.interworking.passpoint.hs2Parameters, pString) == 0){

return TRUE;

}else {

cJSON *p_root = cJSON_Parse(pString);

if(p_root == NULL) {

wifi_util_dbg_print(WIFI_DMCLI,"%s:%d Invalid json for vap %s\n", __FUNCTION__,__LINE__,pcfg->vap_name);

return FALSE;

}

AnscCopyString((char*)vapInfo->u.bss_info.interworking.passpoint.hs2Parameters,pString);

set_dml_cache_vap_config_changed(instance_number - 1);

return TRUE;

}

}

}

if ((Parameters != NULL) && (strcmp(ParamName, "Parameters") == 0))

{

if (strcmp((char*)vapInfo->u.bss_info.interworking.passpoint.hs2Parameters, pString) == 0){

return TRUE;

} else {

cJSON *p_root = cJSON_Parse(pString);

if(p_root == NULL) {

wifi_util_dbg_print(WIFI_DMCLI,"%s:%d Invalid json for vap %s\n", __FUNCTION__,__LINE__,pcfg->vap_name);

return FALSE;

}

AnscCopyString((char*)vapInfo->u.bss_info.interworking.passpoint.hs2Parameters,pString);

set_dml_cache_vap_config_changed(instance_number - 1);

return TRUE;

}

}

Pavlo-Uvarov · 2025-09-22T19:08:42Z

source/dml/tr_181/ml/cosa_wifi_dml.c

+	if(ParamName != NULL) {
+		if (strcmp(ParamName, "Enable") == 0) {    
+	        if (global_wifi_config->global_parameters.mgt_frame_rate_limit_enable == bValue) {
+	            return TRUE;
+	        }
+	        wifi_util_dbg_print(WIFI_DMCLI, "%s:%d: RateLimit Enable: %d\n", __func__, __LINE__,
+	            bValue);
+	        global_wifi_config->global_parameters.mgt_frame_rate_limit_enable = bValue;
+
+	        if (push_global_config_dml_cache_to_one_wifidb() != RETURN_OK) {
+	            wifi_util_error_print(WIFI_DMCLI,
+	                "%s:%d: Failed to push RateLimit Enable value to onewifi db\n", __func__, __LINE__);
+	        }
+	        return TRUE;
+	    }
+	}


Please remove redundant nesting:

Suggested change

if(ParamName != NULL) {

if (strcmp(ParamName, "Enable") == 0) {

if (global_wifi_config->global_parameters.mgt_frame_rate_limit_enable == bValue) {

return TRUE;

}

wifi_util_dbg_print(WIFI_DMCLI, "%s:%d: RateLimit Enable: %d\n", __func__, __LINE__,

bValue);

global_wifi_config->global_parameters.mgt_frame_rate_limit_enable = bValue;

if (push_global_config_dml_cache_to_one_wifidb() != RETURN_OK) {

wifi_util_error_print(WIFI_DMCLI,

"%s:%d: Failed to push RateLimit Enable value to onewifi db\n", __func__, __LINE__);

}

return TRUE;

}

}

if ((ParamName != NULL) && (strcmp(ParamName, "Enable") == 0))

{

if (global_wifi_config->global_parameters.mgt_frame_rate_limit_enable == bValue) {

return TRUE;

}

wifi_util_dbg_print(WIFI_DMCLI, "%s:%d: RateLimit Enable: %d\n", __func__, __LINE__, bValue);

global_wifi_config->global_parameters.mgt_frame_rate_limit_enable = bValue;

if (push_global_config_dml_cache_to_one_wifidb() != RETURN_OK) {

wifi_util_error_print(WIFI_DMCLI,

"%s:%d: Failed to push RateLimit Enable value to onewifi db\n", __func__, __LINE__);

}

return TRUE;

}

Pavlo-Uvarov · 2025-09-22T19:09:32Z

source/dml/tr_181/ml/cosa_wifi_dml.c

@@ -20978,7 +21271,10 @@ BOOL MgtFrameRateLimit_SetParamUlongValue(ANSC_HANDLE hInsContext, char *ParamNa
        return FALSE;
    }

-    if (AnscEqualString(ParamName, "RateLimit", TRUE)) {
+	if(ParamName == NULL) {
+        return FALSE;


Suggested change

return FALSE;

wifi_util_dbg_print(WIFI_DMCLI,"%s:%d Unable to get Global Config\n", __FUNCTION__,__LINE__);

return FALSE;

mateuszCieslak-GL · 2025-09-22T22:44:26Z

source/dml/tr_181/ml/cosa_harvester_dml.c

+    if(ParamName != NULL) {
+	    if (strcmp(ParamName, "Enable") == 0) {
+            pcfg->ActiveMsmtEnable = bValue;
+            push_blaster_config_dml_to_ctrl_queue();
+            return TRUE;
+        }
+	}


or it would be nicer to check ParamName != NULL in the first if in the function, just as you did it in other cases here.

mateuszCieslak-GL · 2025-09-22T22:56:26Z

source/dml/tr_181/sbapi/cosa_wifi_apis.c

-                //ccspWifiDbgPrint(CCSP_WIFI_TRACE, "%s Token : %s txRate : %d\n", __FUNCTION__, token, *pTxRate);
-                isRateInvalid = FALSE;
-            }
+			if(wifiDataTxRateMap[seqCounter].DataTxRateStr != NULL) {


i think this null check is redundant here. This string is filled by the OneWifi, so extreme defensive programming is not needed. imho.

mateuszCieslak-GL · 2025-09-22T22:57:43Z

source/dml/tr_181/sbapi/cosa_wifi_apis.c

-            isregDomainInvalid = FALSE;
-            break;
-        }
+		if((tmp_regDomain != NULL) && (wifiCountryMapMembers[seqCounter].countryStr != NULL)) {


again, redundant check. tmp_regDomain is being worked on 5 lines above.

mateuszCieslak-GL · 2025-09-22T22:58:45Z

source/dml/tr_181/sbapi/cosa_wifi_apis.c

-            isregDomainInvalid = FALSE;
-            break;
-        }
+		if((environment != NULL) && (wifiEnviromentMap[seqCounter].environment != NULL)) {


redundant check(s)

mateuszCieslak-GL · 2025-09-22T23:01:13Z

source/dml/tr_181/sbapi/cosa_wifi_apis.c

                isWifiStdInvalid = FALSE;
            }
-            else if (AnscEqualString(token, wifiStdDmlMap[seqCounter].wifiStdName, TRUE))
+            else if ((wifiStdDmlMap[seqCounter].wifiStdName != NULL) && (strcmp(token, wifiStdDmlMap[seqCounter].wifiStdName) == 0))


unless this is warranted, please stick to pure replacement of function. anscequalstring->strcmp.
ANy performance improvement one might notice can be easily reversed by making redundant checks.

mateuszCieslak-GL · 2025-09-22T23:01:49Z

source/dml/tr_181/sbapi/cosa_wifi_apis.c

-            isBandInvalid = FALSE;
-            break;
-        }
+		if(wifiFreqBandMap[seqCounter].wifiFreqBandStr != NULL) {


mateuszCieslak-GL · 2025-09-22T23:06:11Z

source/dml/tr_181/ml/cosa_wifi_dml.c

please move this file to separate review, and split it in a patch series - at list 2, so that diff is shorter. Ideally < 500-800 changed lines per review.

pradeeptakdas · 2025-10-28T20:01:04Z

Andre will fix the review comments and bring the changes in a separate PR

Sukanya673 requested a review from a team as a code owner June 23, 2025 09:52

Merge branch 'rdkcentral:develop' into 2f45a5

3b9793e

Sukanya673 added 2 commits July 3, 2025 12:02

Merge branch 'develop' into 2f45a5

1883f7f

Merge branch 'develop' into 2f45a5

b25c4be

Sukanya673 marked this pull request as draft August 14, 2025 16:12

Sukanya673 added 9 commits August 16, 2025 00:18

NULL check handled cosa_harvester_dml.c

099b958

null check handled cosa_logging_dml.c

ff06792

null check handled plugin_main.c

63df0db

Update cosa_wifi_apis.c

d19fd98

Update cosa_wifi_dml.c with null handling

816c4fb

Update cosa_wifi_dml.c

59714a5

Update cosa_wifi_dml.c

24f339a

Update cosa_wifi_dml.c

8439efe

Merge branch 'rdkcentral:develop' into 2f45a5

26db813

Sukanya673 marked this pull request as ready for review September 22, 2025 06:50

Pavlo-Uvarov suggested changes Sep 22, 2025

View reviewed changes

mateuszCieslak-GL suggested changes Sep 22, 2025

View reviewed changes

pradeeptakdas closed this Oct 28, 2025

github-actions bot locked and limited conversation to collaborators Oct 28, 2025

		//WiFi_SetHS2Status(vap_pcfg->vap_index, false, true);
		*pBool = interworking_pcfg->passpoint.enable;

	return FALSE;
	wifi_util_dbg_print(WIFI_DMCLI,"%s:%d Unable to get Global Config\n", __FUNCTION__,__LINE__);
	return FALSE;

RDKCOM-5335: RDKBDEV-3195 update AnscEqualString() -> strcmp() #419

RDKCOM-5335: RDKBDEV-3195 update AnscEqualString() -> strcmp() #419

Uh oh!

Conversation

Sukanya673 commented Jun 23, 2025

Uh oh!

narendradandu commented Jul 1, 2025

Uh oh!

CLAassistant commented Aug 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

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!

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!

pradeeptakdas commented Oct 28, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

CLAassistant commented Aug 14, 2025 •

edited

Loading