RDKB-62330 : Selfheal to recover devices when subdoc version mismatch#110
RDKB-62330 : Selfheal to recover devices when subdoc version mismatch#110
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request implements a selfheal mechanism to detect and recover from subdocument version mismatches between the webconfig client and components. The implementation adds new source files that parse webconfig properties, compare versions stored in the webconfig database with component versions in syscfg, and trigger a force reset via RBUS when mismatches are detected.
Changes:
- Added new selfheal module (
webcfg_selfheal.c/h) for version mismatch detection - Integrated the selfheal check into component initialization in
ssp_main.c - Updated build configuration to include new source file and cjson dependency
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 14 comments.
| File | Description |
|---|---|
| source/TandDSsp/webcfg_selfheal.h | Header file defining structures, enums, and function prototypes for webconfig selfheal functionality |
| source/TandDSsp/webcfg_selfheal.c | Implementation of webconfig properties parsing, version comparison, and RBUS force reset mechanism |
| source/TandDSsp/ssp_main.c | Initialization of webconfig properties and version mismatch check at boot |
| source/TandDSsp/Makefile.am | Build system updates to include new source file and cjson library dependency |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Reason for change: Selfheal to recover devices when webconfig client and components have subdoc version mismatch. Test Procedure: Verify that when webconfig client and components have subdoc version mismatch the device should forcerest and recover the device. Priority: P1 Risks: None Signed-off-by: KavyaChowdahalli_Suresh@comcast.com
3f0868f to
6d5dc31
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 19 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| free(p); | ||
| p = n; | ||
| } | ||
| gsdInfoHead = gsdInfoTail = NULL; |
There was a problem hiding this comment.
Variable name mismatch: The function uses 'gsdInfoHead' and 'gsdInfoTail' but the global variables are declared as 'g_sdInfoHead' and 'g_sdInfoTail' (with 'g_' prefix). This will cause a compilation error or access to undeclared variables.
| if (supportedbits) { | ||
| free(supportedbits); | ||
| supportedbits = NULL; | ||
| } | ||
| if (value != NULL) { | ||
| supportedbits = strdup(value); |
There was a problem hiding this comment.
Variable name mismatch: The function uses 'supportedbits' but the global variable is declared as 'supported_bits' (with underscore). This will cause a compilation error or access to an undeclared variable.
| free(p); | ||
| p = n; | ||
| } | ||
| gspInfoHead = gspInfoTail = NULL; |
There was a problem hiding this comment.
Variable name mismatch: The function uses 'gspInfoHead' and 'gspInfoTail' but the global variables are declared as 'g_spInfoHead' and 'g_spInfoTail' (with 'g_' prefix). This will cause a compilation error or access to undeclared variables.
| if (gsdInfoTail == NULL) { | ||
| gsdInfoHead = gsdInfoTail = sdInfo; | ||
| } else { | ||
| gsdInfoTail->next = sdInfo; | ||
| gsdInfoTail = sdInfo; |
There was a problem hiding this comment.
Variable name mismatch: The code uses 'gsdInfoTail' but the global variable is declared as 'g_sdInfoTail' (with 'g_' prefix). This will cause a compilation error or access to an undeclared variable.
| if (gspInfoTail == NULL) { | ||
| gspInfoHead = gspInfoTail = spInfo; | ||
| } else { | ||
| gspInfoTail->next = spInfo; | ||
| gspInfoTail = spInfo; |
There was a problem hiding this comment.
Variable name mismatch: The code uses 'gspInfoTail' but the global variable is declared as 'g_spInfoTail' (with 'g_' prefix). This will cause a compilation error or access to an undeclared variable.
| } | ||
| } | ||
|
|
||
| //SelfHeal Subdoc Version Mismatch |
There was a problem hiding this comment.
Incomplete comment: The comment lacks proper formatting and should follow the project's comment style. Consider using a complete sentence with proper capitalization and punctuation, such as: 'Self-heal subdoc version mismatch detection'.
| //SelfHeal Subdoc Version Mismatch | |
| // Self-heal subdoc version mismatch detection. |
| while(!feof(pipe)) { | ||
| size_t n = fread(buf, 1, sizeof(buf), pipe); | ||
| if(n) { | ||
| char *tmp = (char *)realloc(data, len + n + 1); | ||
| if (!tmp) { | ||
| free(data); | ||
| return NULL; | ||
| } | ||
| data = tmp; | ||
| memcpy(data + len, buf, n); len += n; | ||
| data[len] = '\0'; |
There was a problem hiding this comment.
Potential resource leak: If feof returns true before any data is read, the function returns NULL which is correct. However, if an fread error occurs (ferror would be set), the function continues looping which could lead to an infinite loop. Consider checking ferror(pipe) in addition to feof(pipe) to properly handle read errors.
| while(!feof(pipe)) { | |
| size_t n = fread(buf, 1, sizeof(buf), pipe); | |
| if(n) { | |
| char *tmp = (char *)realloc(data, len + n + 1); | |
| if (!tmp) { | |
| free(data); | |
| return NULL; | |
| } | |
| data = tmp; | |
| memcpy(data + len, buf, n); len += n; | |
| data[len] = '\0'; | |
| for(;;) { | |
| size_t n = fread(buf, 1, sizeof(buf), pipe); | |
| if(n > 0) { | |
| char *tmp = (char *)realloc(data, len + n + 1); | |
| if (!tmp) { | |
| free(data); | |
| return NULL; | |
| } | |
| data = tmp; | |
| memcpy(data + len, buf, n); | |
| len += n; | |
| data[len] = '\0'; | |
| } else { | |
| if (feof(pipe)) { | |
| break; | |
| } | |
| if (ferror(pipe)) { | |
| CcspTraceError(("%s: fread error while reading pipe\n", __FUNCTION__)); | |
| free(data); | |
| return NULL; | |
| } | |
| /* No data read, not EOF, and no error: continue and retry */ |
| if (supportedversion) { | ||
| free(supportedversion); | ||
| supportedversion = NULL; | ||
| } | ||
| if (value != NULL) { | ||
| supportedversion = strdup(value); | ||
| } |
There was a problem hiding this comment.
Variable name mismatch: The function uses 'supportedversion' but the global variable is declared as 'supported_version' (with underscore). This will cause a compilation error or access to an undeclared variable.
| if (supportedbits) { | ||
| free(supportedbits); | ||
| supportedbits = NULL; | ||
| } | ||
| if (supportedversion) { | ||
| free(supportedversion); | ||
| supportedversion = NULL; |
There was a problem hiding this comment.
Variable name mismatch: The code uses 'supportedbits' but the global variable is declared as 'supported_bits' (with underscore). This will cause a compilation error or access to an undeclared variable.
| if (supportedbits) { | |
| free(supportedbits); | |
| supportedbits = NULL; | |
| } | |
| if (supportedversion) { | |
| free(supportedversion); | |
| supportedversion = NULL; | |
| if (supported_bits) { | |
| free(supported_bits); | |
| supported_bits = NULL; | |
| } | |
| if (supported_version) { | |
| free(supported_version); | |
| supported_version = NULL; |
| if (supportedversion) { | ||
| free(supportedversion); | ||
| supportedversion = NULL; |
There was a problem hiding this comment.
Variable name mismatch: The code uses 'supportedversion' but the global variable is declared as 'supported_version' (with underscore). This will cause a compilation error or access to an undeclared variable.
Reason for change: Selfheal to recover devices when webconfig client and components have subdoc version mismatch.
Test Procedure: Verify that when webconfig client and components have subdoc version mismatch the device should forcerest and recover the device. Priority: P1
Risks: None
Signed-off-by: KavyaChowdahalli_Suresh@comcast.com