RDKB-62660: Fix pausing device from Xfinity app is not working #193
Conversation
Reason for change: root@Docsis-Gateway:~# cat /tmp/.pcmd 1 08:9d:f4:2d:54:33 root@Docsis-Gateway:~# When fgets reads the first line "1\n", the newline character is included in the buffer. The strtol function stops parsing at the newline, so endptr points to '\n', not '\0'. Validation check and fails because *endptr != '\0' is true (it's '\n'). Test Procedure: Login to xfinity app. Create a profile and assign a device Pause the assigned device using pause device option Risks: Low Signed-off-by:Veeraputhiran_Thangavel@comcast.com
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where pausing a device from the Xfinity app was not working due to incorrect validation of file input. The issue was that fgets includes the newline character in the buffer, but the validation logic only accepted null terminators, causing valid input to fail validation.
Changes:
- Modified the validation condition in
getPcmdList()to accept both '\0' and '\n' as valid string terminators afterstrtolparsing
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| errno = 0; | ||
| numDev = strtol(buf, &endptr, 10); | ||
| if (endptr == buf || *endptr != '\0' || errno == ERANGE) | ||
| if (endptr == buf || (*endptr != '\0' && *endptr != '\n') || errno == ERANGE) |
There was a problem hiding this comment.
While this fix correctly solves the immediate issue, it deviates from the established pattern in this codebase for handling newlines after fgets. At line 8474, the codebase explicitly strips the newline character after reading with fgets by checking the length and replacing the newline with a null terminator. Consider refactoring to follow this same pattern for consistency: after line 8982, check if the string ends with a newline and replace it with a null terminator before validation. This would make the validation check simpler and more consistent with the rest of the codebase.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
|
@veeraputhiran-thangavel I've opened a new pull request, #198, to work on those changes. Once the pull request is ready, I'll request review from you. |
Reason for change:
root@Docsis-Gateway:
# cat /tmp/.pcmd#1
08:9d:f4:2d:54:33
root@Docsis-Gateway:
When fgets reads the first line "1\n", the newline character is included in the buffer. The strtol function stops parsing at the newline, so endptr points to '\n', not '\0'. Validation check and fails because *endptr != '\0' is true (it's '\n').
Test Procedure:
Login to xfinity app.
Create a profile and assign a device
Pause the assigned device using pause device option
Risks: Low
Signed-off-by:Veeraputhiran_Thangavel@comcast.com