-
Notifications
You must be signed in to change notification settings - Fork 41
RDKB-63166 : Update device.properties file during runtime #189
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
Reason for change : Modify device.properties file based on partner ID Test Procedure : Check that /etc/device.properties contains "IS_BCI=yes" Risks : Low Priority: Medium Signed-off-by: abhishek_kumaracee2@comcast.com
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Updates runtime initialization behavior so that Onestack devices with PartnerID comcast-business have partner-specific defaults written into /etc/device.properties (specifically IS_BCI=yes).
Changes:
- Add helper logic to append partner defaults (currently
IS_BCI=yes) to/etc/device.propertiesfor_ONESTACK_PRODUCT_REQ_. - Invoke the new device.properties update when PartnerID matches
comcast-business.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| FILE *fp = NULL; | ||
| //check file for existence | ||
| if (access(DEV_PROP_FILE, F_OK) != 0) | ||
| { | ||
| APPLY_PRINT("[Utopia - %s]File %s does not exist\n",__FUNCTION__,DEV_PROP_FILE); | ||
| return 1; | ||
| } | ||
|
|
||
| //open for writing | ||
| fp = fopen(DEV_PROP_FILE, "a"); | ||
| if (fp == NULL) | ||
| { | ||
| APPLY_PRINT("[Utopia - %s]File %s could not open for writing\n",__FUNCTION__,DEV_PROP_FILE); | ||
| return 1; | ||
| } | ||
|
|
||
| int i = 0; | ||
| for (i = 0; g_dev_prop_params[i] != NULL; i++) | ||
| { | ||
| if (fprintf(fp, "%s\n", g_dev_prop_params[i]) < 0) | ||
| { | ||
| APPLY_PRINT("[Utopia - %s]File %s write failed\n",__FUNCTION__,DEV_PROP_FILE); | ||
| fclose(fp); | ||
| return 1; | ||
| } | ||
| } | ||
|
|
||
| APPLY_PRINT("[Utopia - %s]File %s write successful\n",__FUNCTION__,DEV_PROP_FILE); | ||
|
|
||
| fclose(fp); | ||
|
|
Copilot
AI
Jan 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Appending "IS_BCI=yes" unconditionally makes this change non-idempotent: every time apply_system_defaults runs (e.g., across reboots) it will add another duplicate line, and it also won’t correct an existing "IS_BCI" key set to a different value. Consider parsing /etc/device.properties to detect an existing IS_BCI entry and either skip the append if already set to "yes" or rewrite the file (temp file + rename) to update/replace the key.
| FILE *fp = NULL; | |
| //check file for existence | |
| if (access(DEV_PROP_FILE, F_OK) != 0) | |
| { | |
| APPLY_PRINT("[Utopia - %s]File %s does not exist\n",__FUNCTION__,DEV_PROP_FILE); | |
| return 1; | |
| } | |
| //open for writing | |
| fp = fopen(DEV_PROP_FILE, "a"); | |
| if (fp == NULL) | |
| { | |
| APPLY_PRINT("[Utopia - %s]File %s could not open for writing\n",__FUNCTION__,DEV_PROP_FILE); | |
| return 1; | |
| } | |
| int i = 0; | |
| for (i = 0; g_dev_prop_params[i] != NULL; i++) | |
| { | |
| if (fprintf(fp, "%s\n", g_dev_prop_params[i]) < 0) | |
| { | |
| APPLY_PRINT("[Utopia - %s]File %s write failed\n",__FUNCTION__,DEV_PROP_FILE); | |
| fclose(fp); | |
| return 1; | |
| } | |
| } | |
| APPLY_PRINT("[Utopia - %s]File %s write successful\n",__FUNCTION__,DEV_PROP_FILE); | |
| fclose(fp); | |
| FILE *in_fp = NULL; | |
| FILE *out_fp = NULL; | |
| char tmp_path[256]; | |
| char line[512]; | |
| bool is_bci_present = false; | |
| /* check file for existence */ | |
| if (access(DEV_PROP_FILE, F_OK) != 0) | |
| { | |
| APPLY_PRINT("[Utopia - %s]File %s does not exist\n", __FUNCTION__, DEV_PROP_FILE); | |
| return 1; | |
| } | |
| /* open original file for reading */ | |
| in_fp = fopen(DEV_PROP_FILE, "r"); | |
| if (in_fp == NULL) | |
| { | |
| APPLY_PRINT("[Utopia - %s]File %s could not open for reading\n", __FUNCTION__, DEV_PROP_FILE); | |
| return 1; | |
| } | |
| /* create temporary file for writing updated content */ | |
| if (snprintf(tmp_path, sizeof(tmp_path), "%s.tmp", DEV_PROP_FILE) >= (int)sizeof(tmp_path)) | |
| { | |
| APPLY_PRINT("[Utopia - %s]Temporary path buffer too small for %s\n", __FUNCTION__, DEV_PROP_FILE); | |
| fclose(in_fp); | |
| return 1; | |
| } | |
| out_fp = fopen(tmp_path, "w"); | |
| if (out_fp == NULL) | |
| { | |
| APPLY_PRINT("[Utopia - %s]File %s could not open for writing\n", __FUNCTION__, tmp_path); | |
| fclose(in_fp); | |
| return 1; | |
| } | |
| /* copy existing lines, normalizing any IS_BCI entry to IS_BCI=yes */ | |
| while (fgets(line, sizeof(line), in_fp) != NULL) | |
| { | |
| if (strncmp(line, "IS_BCI=", 7) == 0) | |
| { | |
| if (fputs("IS_BCI=yes\n", out_fp) == EOF) | |
| { | |
| APPLY_PRINT("[Utopia - %s]File %s write failed while updating IS_BCI\n", __FUNCTION__, tmp_path); | |
| fclose(in_fp); | |
| fclose(out_fp); | |
| unlink(tmp_path); | |
| return 1; | |
| } | |
| is_bci_present = true; | |
| } | |
| else | |
| { | |
| if (fputs(line, out_fp) == EOF) | |
| { | |
| APPLY_PRINT("[Utopia - %s]File %s write failed while copying\n", __FUNCTION__, tmp_path); | |
| fclose(in_fp); | |
| fclose(out_fp); | |
| unlink(tmp_path); | |
| return 1; | |
| } | |
| } | |
| } | |
| /* append any additional parameters, avoiding duplicate IS_BCI entries */ | |
| int i = 0; | |
| for (i = 0; g_dev_prop_params[i] != NULL; i++) | |
| { | |
| const char *param = g_dev_prop_params[i]; | |
| if (strncmp(param, "IS_BCI=", 7) == 0) | |
| { | |
| if (is_bci_present) | |
| { | |
| /* already present and normalized, skip to keep idempotent */ | |
| continue; | |
| } | |
| is_bci_present = true; | |
| } | |
| if (fprintf(out_fp, "%s\n", param) < 0) | |
| { | |
| APPLY_PRINT("[Utopia - %s]File %s write failed\n", __FUNCTION__, tmp_path); | |
| fclose(in_fp); | |
| fclose(out_fp); | |
| unlink(tmp_path); | |
| return 1; | |
| } | |
| } | |
| fclose(in_fp); | |
| if (fclose(out_fp) == EOF) | |
| { | |
| APPLY_PRINT("[Utopia - %s]File %s close failed\n", __FUNCTION__, tmp_path); | |
| unlink(tmp_path); | |
| return 1; | |
| } | |
| if (rename(tmp_path, DEV_PROP_FILE) != 0) | |
| { | |
| APPLY_PRINT("[Utopia - %s]Failed to replace %s with %s\n", __FUNCTION__, DEV_PROP_FILE, tmp_path); | |
| unlink(tmp_path); | |
| return 1; | |
| } | |
| APPLY_PRINT("[Utopia - %s]File %s write successful\n", __FUNCTION__, DEV_PROP_FILE); |
| #if defined (_ONESTACK_PRODUCT_REQ_) | ||
| if (0 == strcasecmp (PartnerID, "comcast-business")) | ||
| { | ||
| if ( 1 == addPartnerDefaultsToDevPropFile()) |
Copilot
AI
Jan 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The caller checks 1 == addPartnerDefaultsToDevPropFile(), which silently ignores failures if the function ever returns any non-1 error code. Since this function follows the common 0 == success convention, it’s safer to check != 0 (or < 0/> 0 consistently) and treat any non-zero as failure (similar to the nearby if (0 != ApplyPartnersObjectItemsIntoSysevents(...))).
| if ( 1 == addPartnerDefaultsToDevPropFile()) | |
| if ( 0 != addPartnerDefaultsToDevPropFile()) |
| fp = fopen(DEV_PROP_FILE, "a"); | ||
| if (fp == NULL) | ||
| { | ||
| APPLY_PRINT("[Utopia - %s]File %s could not open for writing\n",__FUNCTION__,DEV_PROP_FILE); |
Copilot
AI
Jan 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Log message grammar: "File %s could not open for writing" should be phrased as "could not be opened for writing" (or similar) for clarity in logs.
| APPLY_PRINT("[Utopia - %s]File %s could not open for writing\n",__FUNCTION__,DEV_PROP_FILE); | |
| APPLY_PRINT("[Utopia - %s]File %s could not be opened for writing\n",__FUNCTION__,DEV_PROP_FILE); |
| static int addPartnerDefaultsToDevPropFile() | ||
| { | ||
| FILE *fp = NULL; |
Copilot
AI
Jan 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This new no-argument function is declared as addPartnerDefaultsToDevPropFile() instead of addPartnerDefaultsToDevPropFile(void). The rest of this file uses the explicit (void) style for no-arg functions (e.g., set_defaults(void) at apply_system_defaults.c:558), so aligning this signature avoids old-style C function declarations.
Reason for change : Modify device.properties file based on partner ID
Test Procedure : Check that /etc/device.properties contains "IS_BCI=yes"
Risks : Low
Priority: Medium
Signed-off-by: abhishek_kumaracee2@comcast.com