Topic/crashtesting

[Vismalskumar0]

No description provided.

[Copilot]

Pull request overview

This PR updates the hostIf curl write callback to use libcurl’s userdata correctly, adjusts the corresponding unit test call site, and adds more defensive parsing + additional logging in the WiFi EndPoint refreshCache() JSON-RPC path.

Changes:

• Fix writeCurlResponse to append into a caller-provided std::string via void* userdata (instead of passing a std::string by value).
• Update the GTest hook/test to pass the response buffer by pointer.
• Add additional JSON validation and extensive step-by-step logging in hostIf_WiFi_EndPoint::refreshCache() (non-RDKV_NM path).

Reviewed changes

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

File	Description
src/hostif/src/hostIf_utils.cpp	Corrects curl write callback signature/behavior to append into a provided buffer via CURLOPT_WRITEDATA.
src/hostif/src/gtest/gtest_src.cpp	Updates the test invocation to pass userdata correctly for the new callback signature.
src/hostif/profiles/wifi/Device_WiFi_EndPoint.cpp	Adds extra JSON type checks and many new step logs in the NetworkManager JSON-RPC refreshCache path.

Comments suppressed due to low confidence (1)

src/hostif/profiles/wifi/Device_WiFi_EndPoint.cpp:333

• The new STEP-by-STEP INFO logs can add significant log volume in a hot path (refreshCache is called by many getters and may run every second when cache expires). Consider moving these to DEBUG/TRACE or guarding them behind a build/runtime debug flag to avoid production log flooding.

    RDK_LOG (RDK_LOG_INFO, LOG_TR69HOSTIF, "[%s][STEP 1] refreshCache start\n", __FUNCTION__);

    static time_t time_of_last_successful_query = 0;
    static int last_call_status = NOK;
    static std::mutex m;

---

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

[Copilot]

if (response.c_str()) is always true for a std::string (even when empty), so the failure branch is unreachable. Use !response.empty() (or return a status from getJsonRPCData) and log the correct failure reason (this isn't a curl init failure here).

[Copilot]

if (response.c_str()) is always true for a std::string, so this check won't detect empty/failed ConnectedSSID responses and the else path is unreachable. Switch to !response.empty() (or propagate an explicit status) and adjust the error message accordingly.

Suggested change

	if(response.c_str())
	if (!response.empty())

[Copilot]

Logging the connected SSID at INFO (SSIDReference parsed: %s) can leak potentially sensitive user/network information into standard logs. Consider lowering this to DEBUG and/or redacting/hashing the SSID value before logging.

Suggested change

	RDK_LOG (RDK_LOG_INFO, LOG_TR69HOSTIF, "[%s][STEP 15] SSIDReference parsed: %s\n", __FUNCTION__, SSIDReference);
	RDK_LOG (RDK_LOG_DEBUG, LOG_TR69HOSTIF, "[%s][STEP 15] SSIDReference parsed: %s\n", __FUNCTION__, SSIDReference);

[Copilot]

if (response.c_str()) is always true for a std::string, so this check won't detect empty/failed WiFiSignalStrength responses and the else path is unreachable. Use !response.empty() (or return a status from getJsonRPCData) and log the correct failure reason.

[Copilot]

This test only asserts the returned byte count, but doesn't verify that the callback actually appends to the provided response buffer. Add an assertion that response matches the input (and consider also exercising multiple invocations to ensure it appends rather than overwrites).

Suggested change

	EXPECT_EQ(written, nmemb);
	EXPECT_EQ(written, nmemb);
	EXPECT_EQ(response, std::string(input));
	// Verify that multiple invocations append to the response buffer rather than overwriting it
	const char* secondInput = "MoreData";
	size_t nmemb2 = strlen(secondInput);
	size_t written2 = getWriteCurlResponse()((void*)secondInput, size, nmemb2, &response);
	EXPECT_EQ(written2, nmemb2);
	EXPECT_EQ(response, std::string(input) + std::string(secondInput));

[Copilot]

The interface-search loop breaks only when it finds type == "WIFI". If no WIFI entry exists, interface will still point to the last array element, so later code may parse enabled from the wrong interface. Track a foundWifi flag (or reset interface=nullptr when no match is found) so non-WIFI arrays don't produce incorrect results.

Suggested change

	for (int i = 0; i < cJSON_GetArraySize(interfaces); i++) {
	interface = cJSON_GetArrayItem(interfaces, i);
	interfaceType = cJSON_GetObjectItem(interface, "type");
	if (cJSON_IsString(interfaceType) && interfaceType->valuestring && (strcmp(interfaceType->valuestring, "WIFI") == 0))
	break;
	}
	bool foundWifi = false;
	for (int i = 0; i < cJSON_GetArraySize(interfaces); i++) {
	interface = cJSON_GetArrayItem(interfaces, i);
	interfaceType = cJSON_GetObjectItem(interface, "type");
	if (cJSON_IsString(interfaceType) && interfaceType->valuestring && (strcmp(interfaceType->valuestring, "WIFI") == 0)) {
	foundWifi = true;
	break;
	}
	}
	if (!foundWifi) {
	interface = nullptr;
	}

[Copilot]

Because interface can remain non-null even when a WIFI interface wasn't found, this if (interface) block can incorrectly update Enable based on a non-WIFI interface. Gate this block on an explicit "found WIFI" condition instead of pointer non-nullness.