-
Notifications
You must be signed in to change notification settings - Fork 20
RDKB-63415 : Move SelfHeal Scripts to Cron #91
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?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,5 +1,5 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /* | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * If not stated otherwise in this file or this component's Licenses.txt file the | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Check failure on line 2 in source/util_api/ccsp_msg_bus/ccsp_base_api.c
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * following copyright and licenses apply: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Copyright 2015 RDK Management | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -48,6 +48,8 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #include <ccsp_psm_helper.h> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #include "ccsp_trace.h" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #include "secure_wrapper.h" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /* For AnscEqualString */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #include "ansc_platform.h" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -69,6 +71,23 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #define CCSP_CR_ETHWAN_DEVICE_PROFILE_XML_FILE "/usr/ccsp/cr-ethwan-deviceprofile.xml" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #endif | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Declaring selfheal scripts | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| //char *SCRIPT_PATH = "/usr/ccsp/tad/"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| char *SELF_HEAL_SCRIPTS[] = { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "selfheal_aggressive.sh", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "self_heal_connectivity_test.sh", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "resource_monitor.sh" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+76
to
+80
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| //Start/stop cron jobs | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| typedef struct { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const char *name; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const char *key; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| int def_val; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } CronJob; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #define SCRIPT_COUNT (sizeof(SELF_HEAL_SCRIPTS) / sizeof(SELF_HEAL_SCRIPTS[0])) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| typedef struct _component_info { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| char **list; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| int size; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -3715,3 +3734,117 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return CCSP_error_code; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| void extract_command_output (char *cmd, char *out, int len) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| FILE *fp; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| CcspTraceInfo(("%s : Executing command: %s\n", __FUNCTION__, cmd)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| out[0] = 0; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fp = popen (cmd, "r"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (fp) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (fgets (out, len, fp) != NULL) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| //CID 252175 fix - Parse warning (PW.PARAMETER_HIDDEN) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| size_t len_out = strlen (out); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if ((len_out > 0) && (out[len_out - 1] == '\n')) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| out[len_out - 1] = 0; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pclose (fp); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /* Start all self-heal scripts */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| void start_self_heal_scripts() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for (size_t i = 0; i < SCRIPT_COUNT; i++) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| CcspTraceWarning(("%s: Starting %s\n", __FUNCTION__, SELF_HEAL_SCRIPTS[i])); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Uses the shared path and script name | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| v_secure_system("/usr/ccsp/tad/%s &", SELF_HEAL_SCRIPTS[i]); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /* Stop all self-heal scripts */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| void stop_self_heal_scripts() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| char buf[64], cmd[128]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for (size_t i = 0; i < SCRIPT_COUNT; i++) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| snprintf(cmd, sizeof(cmd), "pidof %s", SELF_HEAL_SCRIPTS[i]); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+3768
to
+3770
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| char buf[64], cmd[128]; | |
| for (size_t i = 0; i < SCRIPT_COUNT; i++) { | |
| snprintf(cmd, sizeof(cmd), "pidof %s", SELF_HEAL_SCRIPTS[i]); | |
| char buf[64], cmd[256]; | |
| for (size_t i = 0; i < SCRIPT_COUNT; i++) { | |
| int n = snprintf(cmd, sizeof(cmd), "pidof %s", SELF_HEAL_SCRIPTS[i]); | |
| if (n < 0 || (size_t)n >= sizeof(cmd)) { | |
| CcspTraceWarning(("%s: Command buffer too small for script name '%s'\n", __FUNCTION__, SELF_HEAL_SCRIPTS[i])); | |
| continue; | |
| } |
Copilot
AI
Feb 18, 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.
Using kill -9 with PIDs obtained from pidof is a potential security risk. The PID in 'buf' comes from shell command output without validation, and if 'pidof' returns multiple PIDs (space-separated), only the first will be killed. Additionally, if a PID is reused between the pidof call and the kill command, an unintended process could be terminated. Consider validating the PID format (digits only) and handling multiple PIDs properly, or use pkill/killall with proper signal handling instead.
Copilot
AI
Feb 18, 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.
There's a potential race condition between checking if a script is running (pidof) and killing it. If the process terminates naturally between these two operations, the PID could be reused by another process, leading to killing an unrelated process. While the time window is small, this is a classic TOCTOU (Time-of-check to time-of-use) vulnerability. Consider using more robust process management or checking the process name/path after obtaining the PID.
| CcspTraceWarning(("%s: Stopping %s \n", __FUNCTION__, SELF_HEAL_SCRIPTS[i])); | |
| v_secure_system("kill -9 %s", buf); | |
| /* buf may contain multiple PIDs separated by spaces; validate each before killing */ | |
| char *saveptr = NULL; | |
| char *pid_str = strtok_r(buf, " ", &saveptr); | |
| int any_killed = 0; | |
| while (pid_str != NULL) { | |
| char proc_path[64]; | |
| char cmdline[256]; | |
| long pid = strtol(pid_str, NULL, 10); | |
| if (pid > 0) { | |
| snprintf(proc_path, sizeof(proc_path), "/proc/%ld/cmdline", pid); | |
| FILE *fp = fopen(proc_path, "r"); | |
| if (fp != NULL) { | |
| size_t nread = fread(cmdline, 1, sizeof(cmdline) - 1, fp); | |
| fclose(fp); | |
| cmdline[nread] = '\0'; | |
| /* Verify that this PID still corresponds to the expected script */ | |
| if (strstr(cmdline, SELF_HEAL_SCRIPTS[i]) != NULL) { | |
| CcspTraceWarning(("%s: Stopping %s (pid %ld)\n", __FUNCTION__, SELF_HEAL_SCRIPTS[i], pid)); | |
| v_secure_system("kill -9 %ld", pid); | |
| any_killed = 1; | |
| } else { | |
| CcspTraceWarning(("%s: Skipping pid %ld for %s: command line does not match\n", | |
| __FUNCTION__, pid, SELF_HEAL_SCRIPTS[i])); | |
| } | |
| } else { | |
| CcspTraceWarning(("%s: Could not open %s for pid %ld; process may have exited\n", | |
| __FUNCTION__, proc_path, pid)); | |
| } | |
| } | |
| pid_str = strtok_r(NULL, " ", &saveptr); | |
| } | |
| if (!any_killed) { | |
| CcspTraceWarning(("%s: No matching processes killed for %s\n", __FUNCTION__, SELF_HEAL_SCRIPTS[i])); | |
| } |
Copilot
AI
Feb 18, 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.
Using SIGKILL (kill -9) to terminate scripts is overly aggressive and doesn't allow for graceful shutdown. Scripts cannot catch SIGKILL to clean up resources, close files, or remove lock files. Consider using SIGTERM first and only escalating to SIGKILL if the process doesn't terminate after a timeout period.
| v_secure_system("kill -9 %s", buf); | |
| v_secure_system("kill %s; sleep 5; pidof %s >/dev/null 2>&1 && kill -9 $(pidof %s)", buf, SELF_HEAL_SCRIPTS[i], SELF_HEAL_SCRIPTS[i]); |
Copilot
AI
Feb 18, 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 cmd buffer is sized at 64 bytes but there's no length validation for the 'key' parameter. If a very long key name is passed, snprintf could truncate the command string, potentially creating invalid or unintended syscfg commands. Consider adding a check for the key length or using a larger buffer with proper bounds checking.
| // Format the command string | |
| snprintf(cmd, sizeof(cmd), "syscfg get %s", key); | |
| /* Validate that the formatted command will fit into cmd */ | |
| { | |
| const char *prefix = "syscfg get "; | |
| size_t prefix_len = strlen(prefix); | |
| size_t key_len = strlen(key); | |
| /* Reserve space for prefix, key, and terminating NUL */ | |
| if (prefix_len + key_len + 1 > sizeof(cmd)) { | |
| CcspTraceWarning(("%s: syscfg key too long, using default interval\n", __FUNCTION__)); | |
| return result; | |
| } | |
| } | |
| // Format the command string | |
| int n = snprintf(cmd, sizeof(cmd), "syscfg get %s", key); | |
| if (n < 0 || (size_t)n >= sizeof(cmd)) { | |
| /* snprintf error or truncation; avoid running a malformed command */ | |
| CcspTraceWarning(("%s: failed to format syscfg command, using default interval\n", __FUNCTION__)); | |
| return result; | |
| } |
Copilot
AI
Feb 18, 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 get_interval function uses atol() without validating that the syscfg output is a valid number. If syscfg returns non-numeric data or an error message, atol() will return 0, which might not be a valid interval. Consider adding validation to check if the buffer contains only digits (and optional whitespace) before calling atol(), and log a warning if invalid data is detected.
Copilot
AI
Feb 18, 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 sed pattern uses the script name without proper escaping. If a script name contains special regex characters (like dots in the extensions), this could fail to match correctly. Consider using '\Q...\E' or equivalent to escape the script name, or use a more robust approach like 'sed "//usr/ccsp/tad/$script/d"' with proper escaping of forward slashes.
| v_secure_system("crontab -l 2>/dev/null | sed '/%s/d' | crontab -", script); | |
| v_secure_system("crontab -l 2>/dev/null | grep -vF '/usr/ccsp/tad/%s' | crontab -", script); |
Copilot
AI
Feb 18, 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 update_cron_entry function performs crontab operations without any error checking. If crontab operations fail (e.g., due to permissions, corrupted crontab, or cron daemon not running), the function silently continues. Consider checking the return value of v_secure_system or implementing some form of error handling to detect and log failures.
| // We always remove the old entry to avoid duplicates | |
| v_secure_system("crontab -l 2>/dev/null | sed '/%s/d' | crontab -", script); | |
| // If should_run is true, we append the new timing | |
| if (should_run) { | |
| v_secure_system("(crontab -l 2>/dev/null; echo '*/%lu * * * * /usr/ccsp/tad/%s') | crontab -", | |
| interval, script); | |
| int ret; | |
| /* We always remove the old entry to avoid duplicates */ | |
| ret = v_secure_system("crontab -l 2>/dev/null | sed '/%s/d' | crontab -", script); | |
| if (ret != 0) { | |
| CcspTraceError(("update_cron_entry: failed to remove cron entry for script '%s', ret=%d\n", | |
| script, ret)); | |
| } | |
| /* If should_run is true, we append the new timing */ | |
| if (should_run) { | |
| ret = v_secure_system("(crontab -l 2>/dev/null; echo '*/%lu * * * * /usr/ccsp/tad/%s') | crontab -", | |
| interval, script); | |
| if (ret != 0) { | |
| CcspTraceError(("update_cron_entry: failed to add cron entry for script '%s' " | |
| "(interval=%lu), ret=%d\n", | |
| script, interval, ret)); | |
| } |
Copilot
AI
Feb 18, 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 cron entry format '*/%lu * * * ' may not work correctly for all interval values. The '/N' syntax in cron means "every Nth unit" starting from 0, so intervals don't align with wall-clock times consistently across restarts. For example, */60 in the minute field will only run at minute 0 (once per hour), not every 60 minutes as might be expected. Consider documenting this behavior or using a different scheduling approach if continuous interval-based execution is required.
| // If should_run is true, we append the new timing | |
| if (should_run) { | |
| v_secure_system("(crontab -l 2>/dev/null; echo '*/%lu * * * * /usr/ccsp/tad/%s') | crontab -", | |
| interval, script); | |
| // If should_run is true, we append the new timing | |
| if (should_run) { | |
| /* | |
| * Note on cron semantics: | |
| * - "*/N" in the minute field means "every Nth minute starting from 0", | |
| * not "every N minutes from now". For values >= 60 this can be | |
| * surprising (e.g., "*/60 * * * *" effectively runs at minute 0 | |
| * of every hour). | |
| * | |
| * We therefore: | |
| * - Use "*/N * * * *" only for 1 <= N <= 59 (minute-based). | |
| * - For N that are multiples of 60, convert to hour/day based | |
| * expressions where appropriate. | |
| */ | |
| if (interval < 1) { | |
| // Non-positive intervals are treated as "do not schedule". | |
| return; | |
| } else if (interval < 60) { | |
| // Run every <interval> minutes. | |
| v_secure_system("(crontab -l 2>/dev/null; echo '*/%lu * * * * /usr/ccsp/tad/%s') | crontab -", | |
| interval, script); | |
| } else if (interval % 60 == 0 && interval < 24 * 60) { | |
| // Run every <hours> hours at minute 0. | |
| unsigned long hours = interval / 60; | |
| v_secure_system("(crontab -l 2>/dev/null; echo '0 */%lu * * * /usr/ccsp/tad/%s') | crontab -", | |
| hours, script); | |
| } else if (interval % (24 * 60) == 0) { | |
| // Run every <days> days at midnight. | |
| unsigned long days = interval / (24 * 60); | |
| v_secure_system("(crontab -l 2>/dev/null; echo '0 0 */%lu * * /usr/ccsp/tad/%s') | crontab -", | |
| days, script); | |
| } else { | |
| // Fallback: keep minute-based "*/N" semantics, with the caveat that | |
| // cron counts from minute 0, not from the time this entry is added. | |
| v_secure_system("(crontab -l 2>/dev/null; echo '*/%lu * * * * /usr/ccsp/tad/%s') | crontab -", | |
| interval, script); | |
| } |
Copilot
AI
Feb 18, 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 order of scripts in the manage_self_heal_cron_state arrays (lines 3820-3823) differs from the order in the SELF_HEAL_SCRIPTS global array (lines 76-80). While not strictly a bug, this inconsistency could lead to confusion during maintenance. Consider using the same order in both locations for clarity.
| {"self_heal_connectivity_test.sh", "ConnTest_PingInterval", 60}, | |
| {"resource_monitor.sh", "resource_monitor_interval", 15}, | |
| {"selfheal_aggressive.sh", "AggressiveInterval", 5} | |
| {"selfheal_aggressive.sh", "AggressiveInterval", 5}, | |
| {"self_heal_connectivity_test.sh", "ConnTest_PingInterval", 60}, | |
| {"resource_monitor.sh", "resource_monitor_interval", 15} |
Copilot
AI
Feb 18, 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.
Using hardcoded magic numbers (2 and 3) for loop bounds instead of calculating array sizes makes the code brittle and error-prone. If the array sizes change, these loops will not automatically adjust. Use sizeof() calculations like 'sizeof(recovery_scripts)/sizeof(recovery_scripts[0])' or define constants for these sizes to improve maintainability.
| if (SelfhealCronEnable) { | |
| // Stop Cron Job of Recovery Scripts | |
| for (int i = 0; i < 2; i++) | |
| update_cron_entry(recovery_scripts[i].name, 0, false); | |
| // Start Cron Jobs of Self-Heal Scripts (using syscfg lookup) | |
| for (int i = 0; i < 3; i++) { | |
| unsigned long val = get_interval(self_heal_scripts[i].key, self_heal_scripts[i].def_val); | |
| update_cron_entry(self_heal_scripts[i].name, val, true); | |
| } | |
| } else { | |
| // Stop Cron Job of Self-Heal Scripts | |
| for (int i = 0; i < 3; i++) | |
| update_cron_entry(self_heal_scripts[i].name, 0, false); | |
| // Start Cron Job of Recovery Scripts (using fixed defaults) | |
| for (int i = 0; i < 2; i++) | |
| const int num_self_heal_scripts = (int)(sizeof(self_heal_scripts) / sizeof(self_heal_scripts[0])); | |
| const int num_recovery_scripts = (int)(sizeof(recovery_scripts) / sizeof(recovery_scripts[0])); | |
| if (SelfhealCronEnable) { | |
| // Stop Cron Job of Recovery Scripts | |
| for (int i = 0; i < num_recovery_scripts; i++) | |
| update_cron_entry(recovery_scripts[i].name, 0, false); | |
| // Start Cron Jobs of Self-Heal Scripts (using syscfg lookup) | |
| for (int i = 0; i < num_self_heal_scripts; i++) { | |
| unsigned long val = get_interval(self_heal_scripts[i].key, self_heal_scripts[i].def_val); | |
| update_cron_entry(self_heal_scripts[i].name, val, true); | |
| } | |
| } else { | |
| // Stop Cron Job of Self-Heal Scripts | |
| for (int i = 0; i < num_self_heal_scripts; i++) | |
| update_cron_entry(self_heal_scripts[i].name, 0, false); | |
| // Start Cron Job of Recovery Scripts (using fixed defaults) | |
| for (int i = 0; i < num_recovery_scripts; i++) |
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.
Multiple blank lines at the end of the header file serve no purpose and should be removed to maintain clean code style.