rebase

[Abhinavpv28]

No description provided.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
5 out of 6 committers have signed the CLA.

✅ nhanasi
✅ venkat0557
✅ Abhinavpv28
✅ AravindanNC
✅ Alan-Ryan
❌ naveenkumarhanasi
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[Copilot]

Pull request overview

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

[Copilot]

This test module performs file I/O and overwrites /etc/rrd/remote_debugger.json at import time. If the file is missing or the test runner lacks permissions, collection will fail before any tests run, and it also makes the suite order-dependent. Move this JSON read/update/write logic into a dedicated test (or a setup/fixture) and prefer writing to a temporary copy of the config rather than mutating /etc directly.

[Copilot]

processIssueType() allocates pIssueNode via malloc() but it is never freed on any path (the previous free(pIssueNode) was removed). Since this runs per event, this introduces an unbounded memory leak in the daemon. Ensure pIssueNode is freed before returning (including error paths), and avoid freeing it inside helpers that don't own it.

[Copilot]

rrd_upload_cleanup_source_dir() builds a shell command and calls system("rm -rf %s"). This is vulnerable to command injection if dir_path contains shell metacharacters, and it also violates the project's stated goal of avoiding system() calls where possible. Replace this with a safe recursive deletion implementation (e.g., nftw()/unlinkat() traversal) and validate that dir_path is within an expected base directory before deleting.

[Copilot]

write_tar_header() splits long paths into prefix/name by taking an arbitrary cut (prefix_len = name_len - 101). This doesn't respect tar's requirement that the split occur on a '/' boundary, and can produce archives with invalid/incorrect entry names for paths >100 chars. Prefer splitting at the last '/' that keeps prefix<=155 and name<=100 (or reject paths that can't be represented).

Suggested change

	/* split into prefix and name */
	size_t prefix_len = name_len - 100 - 1; /* leave room for null */
	if (prefix_len > sizeof(hdr.prefix)) {
	RDK_LOG(RDK_LOG_ERROR, LOG_REMDEBUG, "[%s] File name too long: %s (length %zu)\n",
	__FUNCTION__, name, name_len);
	return -1;
	}
	strncpy(hdr.prefix, name, prefix_len);
	hdr.prefix[prefix_len < sizeof(hdr.prefix) ? prefix_len : sizeof(hdr.prefix) - 1] = '\0';
	strncpy(hdr.name, name + prefix_len + 1, sizeof(hdr.name) - 1);
	hdr.name[sizeof(hdr.name) - 1] = '\0';
	/* split into prefix and name at a '/' boundary according to POSIX ustar */
	size_t split_index = (size_t)-1;
	size_t prefix_len = 0;
	size_t name_part_len = 0;
	/* search for the rightmost '/' that yields valid prefix/name lengths */
	for (size_t i = name_len; i > 0; --i) {
	if (name[i - 1] == '/') {
	size_t p_len = i - 1; /* characters before '/' */
	size_t n_len = name_len - i; /* characters after '/' */
	if (p_len > 0 &&
	p_len <= sizeof(hdr.prefix) - 1 &&
	n_len > 0 &&
	n_len <= sizeof(hdr.name) - 1) {
	split_index = i - 1;
	prefix_len = p_len;
	name_part_len = n_len;
	break;
	}
	}
	}
	if (split_index == (size_t)-1) {
	RDK_LOG(RDK_LOG_ERROR, LOG_REMDEBUG, "[%s] File name too long: %s (length %zu)\n",
	__FUNCTION__, name, name_len);
	return -1;
	}
	/* copy prefix and name parts, ensuring null termination */
	strncpy(hdr.prefix, name, prefix_len);
	hdr.prefix[prefix_len] = '\0';
	strncpy(hdr.name, name + split_index + 1, name_part_len);
	hdr.name[name_part_len] = '\0';

[Copilot]

This appends the mock MAC to /tmp/.estb_mac on every run, which will accumulate multiple lines and can break any code that expects a single MAC value. Use overwrite (>) or truncate the file before writing the mock MAC.

Suggested change

	echo "AA:BB:CC:DD:EE:FF" >> /tmp/.estb_mac
	echo "AA:BB:CC:DD:EE:FF" > /tmp/.estb_mac

[Copilot]

Typo in step text: "shopuld" should be "should".

[Copilot]

Typo in enum comment: "Critial" should be "Critical".

[Copilot]

Typo in step text: "shopuld" should be "should".

[Copilot]

The boolean name is_prod_with_override doesn't match its condition (BUILD_TYPE != prod && /opt/dcm.properties exists). This makes the override logic hard to reason about and easy to misread. Rename it to reflect the actual condition (e.g., non_prod_with_opt_dcm_override) or adjust the condition to match the name.

[Copilot]

Pull request overview

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

[Copilot]

pid1 can be an empty list if remotedebugger fails to start (or pidof returns nothing). The later pid1[0] indexing will raise IndexError and hide the real failure. Add an explicit assertion that pid1 is non-empty after the start attempt (and possibly assert pid2 is non-empty too) before comparing PIDs.

[Copilot]

The --enable-L2support option currently sets L2_SUPPORT_FLAG/SUBDIRS_L2_SUPPORT but neither is substituted nor used by any Makefile, so enabling it has no effect. Also, passing --enable-L2support=no hard-errors even though the help says the default is no. Consider (1) AC_SUBST'ing L2_SUPPORT_FLAG and wiring it into AM_CFLAGS/CPPFLAGS, or (2) removing the option if it's not needed, and avoid AC_MSG_ERROR for the 'no' case.

[Copilot]

SCRIPT_SUCCESS/SCRIPT_FAILURE variables are reversed: SCRIPT_SUCCESS is set to the 'upload Failed' log line and SCRIPT_FAILURE to the 'upload Success' line, which makes the printed status misleading. Swap the constants (or rename them) so the variable name matches the log meaning.

[Copilot]

Typo in step text: "shopuld" → "should".

[Copilot]

Typo in step text: "shopuld" → "should".

[Copilot]

rrd_upload_cleanup_source_dir builds a shell command with an unquoted dir_path and passes it to system("rm -rf %s"). If dir_path is ever influenced by external input, this becomes a command-injection risk and can delete unintended paths (spaces/globs). Prefer a non-shell recursive delete (nftw/unlinkat) and/or strictly validate/escape the directory path before deletion.

[Copilot]

The header comment says the MAC buffer must be at least 18 bytes, but the implementation strips colons and only needs 13 bytes (12 hex chars + NUL). Update the comment to match the actual required size (or change the implementation to return the colon-separated form).

[Copilot]

This appends to /tmp/.estb_mac on every run, which can create a multi-line MAC file and break consumers expecting a single MAC value. Use overwrite (>) or ensure the file is truncated before writing.

[Copilot]

Pull request overview

Copilot reviewed 83 out of 84 changed files in this pull request and generated 5 comments.

[Copilot]

Pull request overview

Copilot reviewed 88 out of 89 changed files in this pull request and generated 5 comments.

[Copilot]

Pull request overview

Copilot reviewed 89 out of 90 changed files in this pull request and generated 5 comments.

Comments suppressed due to low confidence (5)

test/functional-tests/tests/test_rrd_profile_data.py:79

• This rbuscli set invocation has the value before the parameter name, unlike the supported rbuscli set <parameter> string <value> form used by the surrounding functional tests. This will not set setProfileData to Device, so the following get does not validate the intended category path.
  test/functional-tests/tests/test_rrd_profile_data.py:100
• This command also uses rbuscli set <value> <parameter> instead of the parameter/type/value ordering used elsewhere. Because the category is never written to setProfileData, this test can pass or fail based on stale state rather than validating the Process selection.
  test/functional-tests/tests/test_rrd_profile_data.py:143
• The negative test uses the same reversed rbuscli set argument order, so it is testing rbuscli's malformed-command handling rather than how setProfileData handles an unknown category. Use the actual profile-data parameter with the string type argument, then assert the expected behavior.
  test/functional-tests/tests/test_rrd_profile_data.py:165
• This empty-string case is also using a malformed rbuscli set command, so the test never reliably writes an empty value to setProfileData. Please use the same rbuscli set <parameter> string "" form as the other RFC tests before validating the get behavior.
  src/rrdIarmEvents.c:230
• The RBUS value initialized here is never released after rbus_set, including the error path. Repeated deep-sleep events will leak RBUS value objects; release the value before any return from this branch.