From 54c45f927a7fdd35cfc2adf11d7b12610587ab0c Mon Sep 17 00:00:00 2001 From: Abhinavpv28 <162570454+Abhinavpv28@users.noreply.github.com> Date: Fri, 13 Mar 2026 03:35:18 +0530 Subject: [PATCH 1/4] RDK-61009 : [RDKE] Port Log Backup Scripts to Source code (#95) * Create backup_logs_requirements.md * Create backup_logs_migration_HLD.md * Create backup_logs_LLD.md * Create backup_logs_flowcharts.md --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- backup_logs/docs/backup_logs_LLD.md | 1029 +++++++++++++++++ backup_logs/docs/backup_logs_migration_HLD.md | 634 ++++++++++ backup_logs/docs/backup_logs_requirements.md | 327 ++++++ .../docs/diagrams/backup_logs_flowcharts.md | 522 +++++++++ 4 files changed, 2512 insertions(+) create mode 100644 backup_logs/docs/backup_logs_LLD.md create mode 100644 backup_logs/docs/backup_logs_migration_HLD.md create mode 100644 backup_logs/docs/backup_logs_requirements.md create mode 100644 backup_logs/docs/diagrams/backup_logs_flowcharts.md diff --git a/backup_logs/docs/backup_logs_LLD.md b/backup_logs/docs/backup_logs_LLD.md new file mode 100644 index 000000000..2e4f5f0ef --- /dev/null +++ b/backup_logs/docs/backup_logs_LLD.md @@ -0,0 +1,1029 @@ +# Low-Level Design: backup_logs.sh Migration to C + +## 1. Overview + +This Low-Level Design (LLD) document provides detailed implementation specifications for migrating the `backup_logs.sh` shell script to C code for embedded RDK systems. + +## 2. Detailed Data Structures + +### 2.1 Core Configuration Structure +```c +/* Using implementation constants */ +#define MAX_SPECIAL_FILES 32 +#define MAX_ERROR_MESSAGE_LEN 256 +#define MAX_FUNCTION_NAME_LEN 64 +#define MAX_DESCRIPTION_LEN 128 +#define MAX_CONDITIONAL_LEN 64 +#define LOG_BACKUP_LOGS "LOG.RDK.BACKUPLOGS" + +/* Main backup configuration structure (matches implementation) */ +typedef struct { + char log_path[PATH_MAX]; + char prev_log_path[PATH_MAX]; + char prev_log_backup_path[PATH_MAX]; + char persistent_path[PATH_MAX]; + bool hdd_enabled; +} backup_config_t; +``` + +### 2.2 File Operation Structures +```c +/* Backup operation types (matches implementation) */ +typedef enum { + BACKUP_OP_MOVE, + BACKUP_OP_COPY, + BACKUP_OP_DELETE +} backup_operation_type_t; + +/* Special file handling structures (matches implementation) */ +typedef enum { + SPECIAL_FILE_COPY = 0, // cp operation + SPECIAL_FILE_MOVE = 1 // mv operation +} special_file_operation_t; + +typedef struct { + char source_path[PATH_MAX]; + char destination_path[PATH_MAX]; + special_file_operation_t operation; + char conditional_check[MAX_CONDITIONAL_LEN]; +} special_file_entry_t; + +typedef struct { + special_file_entry_t entries[MAX_SPECIAL_FILES]; + size_t count; + bool config_loaded; +} special_files_config_t; + +/* Configuration flags for advanced options */ +typedef struct { + bool debug_enabled; + bool force_rotation; + bool skip_disk_check; + bool cleanup_enabled; +} backup_flags_t; +``` + +### 2.3 Error Handling Structures +```c +/* Return codes (matches implementation) */ +typedef enum { + BACKUP_SUCCESS = 0, + BACKUP_ERROR_CONFIG = -1, + BACKUP_ERROR_FILESYSTEM = -2, + BACKUP_ERROR_PERMISSIONS = -3, + BACKUP_ERROR_MEMORY = -4, + BACKUP_ERROR_INVALID_PARAM = -5, + BACKUP_ERROR_NOT_FOUND = -6, + BACKUP_ERROR_DISK_FULL = -7, + BACKUP_ERROR_SYSTEM = -8 +} backup_result_t; + +/* Error information structure (matches implementation) */ +typedef struct { + int error_code; + char error_message[MAX_ERROR_MESSAGE_LEN]; + char function_name[MAX_FUNCTION_NAME_LEN]; + int line_number; +} error_info_t; +``` + +### 2.4 Backup Level Tracking +```c +typedef enum { + BACKUP_LEVEL_NONE = -1, + BACKUP_LEVEL_BASE = 0, + BACKUP_LEVEL_BAK1 = 1, + BACKUP_LEVEL_BAK2 = 2, + BACKUP_LEVEL_BAK3 = 3 +} backup_level_t; + +typedef struct backup_state { + backup_level_t current_level; + bool has_existing_backup; + char timestamp_str[32]; // Format: MM-DD-YY-HH-MM-SSAM +} backup_state_t; +``` + +## 3. Module Interface Definitions + +### 3.1 Configuration Manager Module +```c +// config_manager.h (actual implementation interfaces) + +// Load backup configuration from RDK property system +int config_load(backup_config_t* config); + +// Validate loaded configuration +int config_validate(const backup_config_t* config); + +// Get specific configuration values +const char* config_get_log_path(void); +bool config_is_hdd_enabled(void); + +// Special files configuration interface +int special_files_load_config(special_files_config_t* config, const char* config_file); +int special_files_validate_config(const special_files_config_t* config); +void special_files_free_config(special_files_config_t* config); +int special_files_execute_all(const special_files_config_t* config, + const backup_config_t* backup_config); +``` + +### 3.2 Directory Manager Module +```c +// Using RDK system utilities (actual implementation) + +// Create directory if not exists +int createDir(char* path); + +// Check if directory exists +bool dir_exists(const char* path); + +// Clean directory contents +int emptyFolder(char* path); + +// Create all required backup directories +int dir_create_workspace(const backup_config_t* config); + +// Validate directory permissions +int dir_check_permissions(const char* path, int required_perms); +``` + +### 3.3 File Operations Module +```c +// Using RDK system utilities (actual implementation) + +// Check file existence +int filePresentCheck(const char* path); + +// Copy file with verification +int copyFiles(const char* source, const char* dest); + +// Remove file safely +int removeFile(const char* path); + +// Find files matching pattern (implemented in backup engine) +int move_log_files_by_pattern(const char* source_dir, const char* dest_dir); + +// Pattern matching for log files +bool matches_log_pattern(const char* filename); // *.txt*, *.log*, bootlog +``` + +### 3.4 Backup Engine Module +```c +// backup_engine.h (actual implementation) + +// Execute HDD-enabled backup strategy +int backup_execute_hdd_enabled_strategy(const backup_config_t* config); + +// Execute HDD-disabled backup strategy +int backup_execute_hdd_disabled_strategy(const backup_config_t* config); + +// Execute common operations (special files, version files, notifications) +int backup_execute_common_operations(const backup_config_t* config); + +// Helper function to move log files by pattern +int move_log_files_by_pattern(const char* source_dir, const char* dest_dir, + char* timestamp_dir, size_t dir_size); + +// Create last_reboot marker file +int backup_create_reboot_marker(const char* directory); + +// Remove old reboot markers +int backup_remove_old_markers(const char* directory); + +// Cleanup backup engine resources +void backup_cleanup(void); +``` + +### 3.5 System Integration Module +```c +// sys_integration.h (actual implementation) + +// Initialize system integration +int sys_init(void); + +// Send systemd notification +int sys_notify_ready(void); +int sys_notify_status(const char* status); + +// Execute external script safely +int sys_execute_disk_check(void); + +// Create persistent marker file +int sys_create_marker(const char* path); + +// Cleanup system integration resources +void sys_cleanup(void); +``` + +### 3.6 RDK Logger Integration Module +```c +// RDK Logger integration (actual implementation) + +#ifdef RDK_LOGGER_EXT +#define RDK_LOGGER_ENABLED +#endif + +#ifdef RDK_LOGGER_ENABLED +#include "rdk_debug.h" +#include "rdk_logger.h" +#endif + +// Logger initialization +int backup_logs_init_logger(void); + +// Extended logger configuration +typedef struct { + char* pModuleName; + int loglevel; + int output; + int format; + void* pFilePolicy; +} rdk_logger_ext_config_t; + +// RDK Logger constants +#define LOG_BACKUP_LOGS "LOG.RDK.BACKUPLOGS" +#define DEBUG_INI_NAME "/etc/debug.ini" +#define BACKUP_LOGS_VERSION "1.0.0" +#define BACKUP_LOGS_BUILD_DATE __DATE__ + +// Logger convenience macros +#define RDK_LOG(level, module, format, ...) \ + rdk_log(level, module, format, ##__VA_ARGS__) +``` + +### 3.7 Special Files Manager Module +```c +// special_files.h (actual implementation) + +// Initialize special files manager +int special_files_init(void); + +// Cleanup special files manager +void special_files_cleanup(void); + +// Load special files configuration (one filename per line) +int special_files_load_config(special_files_config_t* config, const char* config_file); + +// Validate special file entry +int special_files_validate_entry(const special_file_entry_t* entry); + +// Execute single special file operation +int special_files_execute_entry(const special_file_entry_t* entry, + const backup_config_t* backup_config); + +// Execute all special file operations +int special_files_execute_all(const special_files_config_t* config, + const backup_config_t* backup_config); +``` + +## 4. Build System and Dependencies + +### 4.1 Build Configuration +```makefile +# Required libraries and flags (from Makefile.am) +backup_logs_LDADD = -lm -lrdkloggers -lfwutils -lsystemd +backup_logs_CPPFLAGS = -DRDK_LOGGER_EXT +``` + +### 4.2 Dependencies +- **librdkloggers**: RDK logging framework +- **libfwutils**: RDK firmware utilities for configuration and system operations +- **libsystemd**: Systemd integration for service notifications +- **libm**: Math library for numerical operations + +### 4.3 Build-time Configuration +```c +#ifdef RDK_LOGGER_EXT +#define RDK_LOGGER_ENABLED +#endif +``` + +## 5. Detailed Algorithms + +### 5.1 RDK Configuration Loading Algorithm (Actual Implementation) +```c +int config_load_rdk_properties(backup_config_t* config) { + char buffer[PATH_MAX]; + + // Load LOG_PATH from include properties + if (getIncludePropertyData("LOG_PATH", buffer, sizeof(buffer)) == 0) { + strncpy(config->log_path, buffer, sizeof(config->log_path) - 1); + } else { + // Use default if not found + strcpy(config->log_path, "/opt/logs"); + } + + // Load HDD_ENABLED from device properties + if (getDevicePropertyData("HDD_ENABLED", buffer, sizeof(buffer)) == 0) { + config->hdd_enabled = (strcmp(buffer, "true") == 0); + } else { + config->hdd_enabled = false; // Default to false + } + + // Construct derived paths + snprintf(config->prev_log_path, sizeof(config->prev_log_path), + "%s/PreviousLogs", config->log_path); + snprintf(config->prev_log_backup_path, sizeof(config->prev_log_backup_path), + "%s/PreviousLogs_backup", config->log_path); + + return BACKUP_SUCCESS; +} +``` +### 4.2 HDD-Disabled Backup Level Detection +```c +backup_level_t backup_detect_level_hdd_disabled(const backup_config_t* config) { + char filepath[MAX_PATH_LEN]; + + // Check for messages.txt (base level) + snprintf(filepath, sizeof(filepath), "%s/messages.txt", config->prev_log_path); + if (!fileops_exists(filepath)) { + return BACKUP_LEVEL_NONE; + } + + // Check for bak1_messages.txt + snprintf(filepath, sizeof(filepath), "%s/bak1_messages.txt", config->prev_log_path); + if (!fileops_exists(filepath)) { + return BACKUP_LEVEL_BASE; + } + + // Check for bak2_messages.txt + snprintf(filepath, sizeof(filepath), "%s/bak2_messages.txt", config->prev_log_path); + if (!fileops_exists(filepath)) { + return BACKUP_LEVEL_BAK1; + } + + // Check for bak3_messages.txt + snprintf(filepath, sizeof(filepath), "%s/bak3_messages.txt", config->prev_log_path); + if (!fileops_exists(filepath)) { + return BACKUP_LEVEL_BAK2; + } + + return BACKUP_LEVEL_BAK3; // All levels exist, need rotation +} +``` + +### 4.3 File Pattern Matching Algorithm +```c +int fileops_find_pattern_impl(const char* directory, const char* pattern, + file_list_t* results) { + DIR* dir = opendir(directory); + if (!dir) { + return -1; + } + + struct dirent* entry; + results->count = 0; + + while ((entry = readdir(dir)) != NULL && results->count < results->capacity) { + // Skip . and .. + if (strcmp(entry->d_name, ".") == 0 || strcmp(entry->d_name, "..") == 0) { + continue; + } + + // Check if filename matches any of the patterns + bool matches = false; + + // Support multiple patterns: *.txt*, *.log*, *.bin*, bootlog + if (strstr(pattern, "*.txt*") && + (strstr(entry->d_name, ".txt") || strstr(entry->d_name, ".TXT"))) { + matches = true; + } else if (strstr(pattern, "*.log*") && + (strstr(entry->d_name, ".log") || strstr(entry->d_name, ".LOG"))) { + matches = true; + } else if (strstr(pattern, "*.bin*") && + (strstr(entry->d_name, ".bin") || strstr(entry->d_name, ".BIN"))) { + matches = true; + } else if (strstr(pattern, "bootlog") && + strcmp(entry->d_name, "bootlog") == 0) { + matches = true; + } + + if (matches) { + // Build full path + snprintf(results->files[results->count].filename, + sizeof(results->files[results->count].filename), + "%s", entry->d_name); + snprintf(results->files[results->count].source_path, + sizeof(results->files[results->count].source_path), + "%s/%s", directory, entry->d_name); + results->count++; + } + } + + closedir(dir); + return results->count; +} +``` + +### 4.4 Log Rotation Algorithm for HDD-Disabled Devices +```c +int backup_rotate_files_hdd_disabled(const backup_config_t* config) { + char source_path[MAX_PATH_LEN]; + char dest_path[MAX_PATH_LEN]; + file_list_t file_list = {0}; + file_list.capacity = MAX_FILES_PER_DIR; + + // Step 1: Move bak1_ files to base names (bak1_messages.txt -> messages.txt) + if (fileops_find_pattern(config->prev_log_path, "bak1_*", &file_list) > 0) { + for (int i = 0; i < file_list.count; i++) { + // Remove bak1_ prefix + const char* base_name = file_list.files[i].filename + 5; // Skip "bak1_" + snprintf(dest_path, sizeof(dest_path), "%s/%s", + config->prev_log_path, base_name); + + if (fileops_move(file_list.files[i].source_path, dest_path) != 0) { + LOG_ERROR(&error_ctx, ERROR_FILESYSTEM, + "Failed to rotate bak1 file to base"); + return -1; + } + } + } + + // Step 2: Move bak2_ files to bak1_ names + file_list.count = 0; // Reset list + if (fileops_find_pattern(config->prev_log_path, "bak2_*", &file_list) > 0) { + for (int i = 0; i < file_list.count; i++) { + // Replace bak2_ with bak1_ + const char* base_name = file_list.files[i].filename + 5; // Skip "bak2_" + snprintf(dest_path, sizeof(dest_path), "%s/bak1_%s", + config->prev_log_path, base_name); + + if (fileops_move(file_list.files[i].source_path, dest_path) != 0) { + LOG_ERROR(&error_ctx, ERROR_FILESYSTEM, + "Failed to rotate bak2 file to bak1"); + return -1; + } + } + } + + // Step 3: Move bak3_ files to bak2_ names + file_list.count = 0; // Reset list + if (fileops_find_pattern(config->prev_log_path, "bak3_*", &file_list) > 0) { + for (int i = 0; i < file_list.count; i++) { + // Replace bak3_ with bak2_ + const char* base_name = file_list.files[i].filename + 5; // Skip "bak3_" + snprintf(dest_path, sizeof(dest_path), "%s/bak2_%s", + config->prev_log_path, base_name); + + if (fileops_move(file_list.files[i].source_path, dest_path) != 0) { + LOG_ERROR(&error_ctx, ERROR_FILESYSTEM, + "Failed to rotate bak3 file to bak2"); + return -1; + } + } + } + + // Step 4: Move current logs to bak3_ names + file_list.count = 0; // Reset list + char log_patterns[] = "*.txt*,*.log*,*.bin*,bootlog"; + if (fileops_find_pattern(config->log_path, log_patterns, &file_list) > 0) { + for (int i = 0; i < file_list.count; i++) { + snprintf(dest_path, sizeof(dest_path), "%s/bak3_%s", + config->prev_log_path, file_list.files[i].filename); + + if (fileops_move(file_list.files[i].source_path, dest_path) != 0) { + LOG_ERROR(&error_ctx, ERROR_FILESYSTEM, + "Failed to move current log to bak3"); + return -1; + } + } + } + + return 0; // Success +} +``` + +### 4.5 Timestamp Generation Algorithm +```c +int sysint_generate_timestamp(char* buffer, size_t buffer_size) { + time_t raw_time; + struct tm* time_info; + + // Get current time + time(&raw_time); + time_info = localtime(&raw_time); + + if (!time_info) { + return -1; + } + + // Format: MM-DD-YY-HH-MM-SSAM (e.g., 03-05-26-02-30-45PM) + char am_pm = (time_info->tm_hour >= 12) ? 'P' : 'A'; + int hour_12 = time_info->tm_hour; + if (hour_12 == 0) { + hour_12 = 12; // 12 AM + } else if (hour_12 > 12) { + hour_12 -= 12; // Convert to 12-hour format + } + + int bytes_written = snprintf(buffer, buffer_size, + "%02d-%02d-%02d-%02d-%02d-%02d%cM", + time_info->tm_mon + 1, // Month (1-12) + time_info->tm_mday, // Day (1-31) + time_info->tm_year % 100, // Year (2-digit) + hour_12, // Hour (1-12) + time_info->tm_min, // Minute (0-59) + time_info->tm_sec, // Second (0-59) + am_pm); // AM/PM + + if (bytes_written < 0 || bytes_written >= buffer_size) { + return -1; // Buffer overflow or formatting error + } + + return 0; // Success +} +``` + +## 5. Error Handling Implementation + +### 5.1 Error Context Management +```c +static error_context_t g_last_error = {0}; + +void logger_set_error(error_context_t* context, error_code_t code, + const char* function, int line, const char* message) { + if (!context) { + context = &g_last_error; + } + + context->code = code; + context->function_name = function; + context->line_number = line; + time(&context->timestamp); + + // Copy message safely + if (message) { + strncpy(context->message, message, sizeof(context->message) - 1); + context->message[sizeof(context->message) - 1] = '\0'; + } else { + context->message[0] = '\0'; + } +} + +int logger_error(const error_context_t* context) { + struct tm* time_info = localtime(&context->timestamp); + char time_str[64]; + + strftime(time_str, sizeof(time_str), "%Y-%m-%d %H:%M:%S", time_info); + + fprintf(stderr, "[%s] ERROR %d in %s:%d: %s\n", + time_str, context->code, context->function_name, + context->line_number, context->message); + + return context->code; +} +``` + +### 5.2 Recovery Strategies +```c +int backup_recover_from_partial_state(const backup_config_t* config) { + // Check for incomplete operations by looking for temporary files + file_list_t temp_files = {0}; + temp_files.capacity = MAX_FILES_PER_DIR; + + // Look for .tmp, .bak, or other temporary extensions + if (fileops_find_pattern(config->log_path, "*.tmp", &temp_files) > 0) { + LOG_WARN("Found %d temporary files, attempting recovery", temp_files.count); + + for (int i = 0; i < temp_files.count; i++) { + // Try to determine original filename + char original_name[MAX_PATH_LEN]; + strncpy(original_name, temp_files.files[i].filename, + strlen(temp_files.files[i].filename) - 4); // Remove .tmp + original_name[strlen(temp_files.files[i].filename) - 4] = '\0'; + + char original_path[MAX_PATH_LEN]; + snprintf(original_path, sizeof(original_path), "%s/%s", + config->log_path, original_name); + + // If original doesn't exist, restore from temp + if (!fileops_exists(original_path)) { + if (fileops_move(temp_files.files[i].source_path, original_path) == 0) { + LOG_INFO("Recovered file: %s", original_name); + } + } else { + // Original exists, remove temp file + fileops_remove(temp_files.files[i].source_path); + } + } + } + + // Check for incomplete backup directories + DIR* dir = opendir(config->prev_log_path); + if (dir) { + struct dirent* entry; + while ((entry = readdir(dir)) != NULL) { + // Look for directories with .incomplete suffix + if (strstr(entry->d_name, ".incomplete")) { + char incomplete_path[MAX_PATH_LEN]; + snprintf(incomplete_path, sizeof(incomplete_path), "%s/%s", + config->prev_log_path, entry->d_name); + + LOG_WARN("Found incomplete backup directory: %s", incomplete_path); + // Remove incomplete backup directory + dir_cleanup(incomplete_path, "*"); + rmdir(incomplete_path); + } + } + closedir(dir); + } + + return 0; +} +``` + +## 6. Memory Management Strategy + +### 6.1 Fixed Buffer Pool Implementation +```c +#define BUFFER_POOL_SIZE 10 +#define BUFFER_SIZE 4096 + +static struct { + char buffers[BUFFER_POOL_SIZE][BUFFER_SIZE]; + bool in_use[BUFFER_POOL_SIZE]; + int allocated_count; +} g_buffer_pool = {0}; + +char* buffer_pool_allocate(void) { + for (int i = 0; i < BUFFER_POOL_SIZE; i++) { + if (!g_buffer_pool.in_use[i]) { + g_buffer_pool.in_use[i] = true; + g_buffer_pool.allocated_count++; + return g_buffer_pool.buffers[i]; + } + } + return NULL; // Pool exhausted +} + +void buffer_pool_free(char* buffer) { + for (int i = 0; i < BUFFER_POOL_SIZE; i++) { + if (g_buffer_pool.buffers[i] == buffer) { + g_buffer_pool.in_use[i] = false; + g_buffer_pool.allocated_count--; + return; + } + } +} + +int buffer_pool_get_usage(void) { + return g_buffer_pool.allocated_count; +} +``` + +### 6.2 Stack-based File Operation +```c +int fileops_move_safe(const char* source, const char* dest) { + char temp_dest[MAX_PATH_LEN]; // Stack allocation + error_context_t error_ctx = {0}; // Stack allocation + + // Create temporary destination name + snprintf(temp_dest, sizeof(temp_dest), "%s.tmp", dest); + + // Step 1: Copy to temporary location + if (fileops_copy(source, temp_dest) != 0) { + LOG_ERROR(&error_ctx, ERROR_FILESYSTEM, "Failed to copy to temporary location"); + return -1; + } + + // Step 2: Verify copy integrity + if (fileops_get_size(source) != fileops_get_size(temp_dest)) { + fileops_remove(temp_dest); + LOG_ERROR(&error_ctx, ERROR_FILESYSTEM, "File size mismatch after copy"); + return -1; + } + + // Step 3: Atomic rename + if (rename(temp_dest, dest) != 0) { + fileops_remove(temp_dest); + LOG_ERROR(&error_ctx, ERROR_FILESYSTEM, "Failed to rename to final destination"); + return -1; + } + + // Step 4: Remove original + if (fileops_remove(source) != 0) { + // Log warning but don't fail the operation + LOG_WARN("Failed to remove source file: %s", source); + } + + return 0; // Success +} +``` + +## 7. Performance Optimization Techniques + +### 7.1 Batch File Operations +```c +int fileops_batch_move(const file_list_t* file_list, const char* dest_dir) { + int success_count = 0; + int total_files = file_list->count; + + // Pre-allocate destination paths to avoid repeated allocations + char dest_paths[MAX_FILES_PER_DIR][MAX_PATH_LEN]; + + // Prepare all destination paths first + for (int i = 0; i < total_files; i++) { + snprintf(dest_paths[i], sizeof(dest_paths[i]), "%s/%s", + dest_dir, file_list->files[i].filename); + } + + // Execute moves in batch with progress tracking + for (int i = 0; i < total_files; i++) { + if (fileops_move_safe(file_list->files[i].source_path, dest_paths[i]) == 0) { + success_count++; + } else { + LOG_WARN("Failed to move file %d of %d: %s", + i + 1, total_files, file_list->files[i].filename); + } + + // Report progress every 100 files for large operations + if (total_files > 100 && (i + 1) % 100 == 0) { + LOG_INFO("Moved %d of %d files (%d%%)", success_count, i + 1, + ((i + 1) * 100) / total_files); + } + } + + LOG_INFO("Batch move completed: %d of %d files successful", + success_count, total_files); + + return (success_count == total_files) ? 0 : -1; +} +``` + +### 7.2 Efficient Directory Traversal +```c +int fileops_find_pattern_optimized(const char* directory, const char* pattern, + file_list_t* results) { + DIR* dir = opendir(directory); + if (!dir) { + return -1; + } + + struct dirent* entry; + results->count = 0; + + // Pre-compile pattern matching criteria for efficiency + bool match_txt = strstr(pattern, "*.txt*") != NULL; + bool match_log = strstr(pattern, "*.log*") != NULL; + bool match_bin = strstr(pattern, "*.bin*") != NULL; + bool match_bootlog = strstr(pattern, "bootlog") != NULL; + + while ((entry = readdir(dir)) != NULL && results->count < results->capacity) { + // Quick checks first (most common rejects) + if (entry->d_name[0] == '.') { + continue; // Skip hidden files and . / .. + } + + bool matches = false; + const char* name = entry->d_name; + size_t name_len = strlen(name); + + // Optimized pattern matching + if (match_bootlog && name_len == 7 && strcmp(name, "bootlog") == 0) { + matches = true; + } else if (name_len >= 4) { // Minimum length for extensions + // Check extensions efficiently + if (match_txt && (strcasestr(name, ".txt") != NULL)) { + matches = true; + } else if (match_log && (strcasestr(name, ".log") != NULL)) { + matches = true; + } else if (match_bin && (strcasestr(name, ".bin") != NULL)) { + matches = true; + } + } + + if (matches) { + // Use pointer arithmetic for efficiency + snprintf(results->files[results->count].filename, MAX_PATH_LEN, "%s", name); + snprintf(results->files[results->count].source_path, MAX_PATH_LEN, + "%s/%s", directory, name); + results->count++; + } + } + + closedir(dir); + return results->count; +} +``` + +## 8. Resource Management + +### 8.1 Resource Cleanup Framework +```c +#define MAX_CLEANUP_HANDLERS 16 + +typedef struct cleanup_handler { + void (*cleanup_func)(void*); + void* resource; + bool in_use; +} cleanup_handler_t; + +static cleanup_handler_t g_cleanup_handlers[MAX_CLEANUP_HANDLERS]; + +int register_cleanup(void (*cleanup_func)(void*), void* resource) { + int i; + + if (cleanup_func == NULL) { + return -1; + } + + for (i = 0; i < MAX_CLEANUP_HANDLERS; ++i) { + if (!g_cleanup_handlers[i].in_use) { + g_cleanup_handlers[i].cleanup_func = cleanup_func; + g_cleanup_handlers[i].resource = resource; + g_cleanup_handlers[i].in_use = true; + return 0; + } + } + + /* No free slot available */ + return -1; +} + +void execute_all_cleanup(void) { + int i; + + for (i = 0; i < MAX_CLEANUP_HANDLERS; ++i) { + if (g_cleanup_handlers[i].in_use && g_cleanup_handlers[i].cleanup_func != NULL) { + g_cleanup_handlers[i].cleanup_func(g_cleanup_handlers[i].resource); + g_cleanup_handlers[i].cleanup_func = NULL; + g_cleanup_handlers[i].resource = NULL; + g_cleanup_handlers[i].in_use = false; + } + } +} + +// Signal handler for graceful shutdown +void signal_handler(int sig) { + LOG_INFO("Received signal %d, cleaning up resources", sig); + execute_all_cleanup(); + exit(sig); +} +``` + +### 8.2 File Descriptor Management +```c +#define MAX_OPEN_FILES 64 + +static struct { + FILE* handles[MAX_OPEN_FILES]; + char paths[MAX_OPEN_FILES][MAX_PATH_LEN]; + int count; +} g_file_registry = {0}; + +FILE* managed_fopen(const char* path, const char* mode) { + if (g_file_registry.count >= MAX_OPEN_FILES) { + LOG_ERROR(NULL, ERROR_RESOURCE, "Too many open files"); + return NULL; + } + + FILE* fp = fopen(path, mode); + if (fp) { + g_file_registry.handles[g_file_registry.count] = fp; + strncpy(g_file_registry.paths[g_file_registry.count], path, MAX_PATH_LEN - 1); + g_file_registry.count++; + } + + return fp; +} + +void managed_fclose_all(void) { + for (int i = 0; i < g_file_registry.count; i++) { + if (g_file_registry.handles[i]) { + fclose(g_file_registry.handles[i]); + g_file_registry.handles[i] = NULL; + } + } + g_file_registry.count = 0; +} +``` + +## 9. Main Program Structure + +### 9.1 Main Function Implementation +```c +int main(int argc, char* argv[]) { + backup_config_t config = {0}; + error_context_t error_ctx = {0}; + int exit_code = ERROR_SUCCESS; + + // Setup signal handlers for graceful shutdown + signal(SIGINT, signal_handler); + signal(SIGTERM, signal_handler); + + do { + // Initialize all subsystems + if (logger_init("backup_logs") != 0) { + fprintf(stderr, "Failed to initialize logging system\n"); + exit_code = ERROR_SYSTEM; + break; + } + + if (config_init() != 0) { + LOG_ERROR(&error_ctx, ERROR_SYSTEM, "Failed to initialize configuration system"); + exit_code = ERROR_SYSTEM; + break; + } + + // Load and validate configuration + if (config_load(&config) != 0) { + LOG_ERROR(&error_ctx, ERROR_CONFIG_INVALID, "Failed to load configuration"); + exit_code = ERROR_CONFIG_INVALID; + break; + } + + if (config_validate(&config) != 0) { + LOG_ERROR(&error_ctx, ERROR_CONFIG_INVALID, "Configuration validation failed"); + exit_code = ERROR_CONFIG_INVALID; + break; + } + + LOG_INFO("Configuration loaded successfully"); + + // Create workspace directories + if (dir_create_workspace(&config) != 0) { + LOG_ERROR(&error_ctx, ERROR_FILESYSTEM, "Failed to create workspace directories"); + exit_code = ERROR_FILESYSTEM; + break; + } + + // Check disk threshold + if (sysint_check_disk_threshold() != 0) { + LOG_WARN("Disk threshold check failed or reported issues"); + // Continue execution - not a fatal error + } + + // Attempt recovery from any partial state + if (backup_recover_from_partial_state(&config) != 0) { + LOG_WARN("Partial state recovery had issues"); + // Continue execution + } + + // Execute appropriate backup strategy + if (config.hdd_enabled) { + LOG_INFO("Executing HDD-enabled backup strategy"); + if (backup_execute_hdd_enabled(&config) != 0) { + LOG_ERROR(&error_ctx, ERROR_FILESYSTEM, "HDD-enabled backup failed"); + exit_code = ERROR_FILESYSTEM; + break; + } + } else { + LOG_INFO("Executing HDD-disabled backup strategy"); + if (backup_execute_hdd_disabled(&config) != 0) { + LOG_ERROR(&error_ctx, ERROR_FILESYSTEM, "HDD-disabled backup failed"); + exit_code = ERROR_FILESYSTEM; + break; + } + } + + // Clean current log directory + if (dir_cleanup(config.log_path, "*.txt*,*.log*,*-*-*-*-*M-") != 0) { + LOG_WARN("Failed to clean current log directory"); + // Continue - not fatal + } + + // Copy version files + if (backup_copy_version_files(&config) != 0) { + LOG_WARN("Failed to copy some version files"); + // Continue - not fatal + } + + // Handle special log files + if (backup_handle_special_files(&config) != 0) { + LOG_WARN("Failed to handle some special log files"); + // Continue - not fatal + } + + // Create persistent marker + if (sysint_create_persistent_marker(config.persistent_path) != 0) { + LOG_WARN("Failed to create persistent marker"); + // Continue - not fatal + } + + // Send systemd notification + if (sysint_notify_systemd("Logs Backup Done..!") != 0) { + LOG_WARN("Failed to send systemd notification"); + // Continue - not fatal + } + + LOG_INFO("Backup operation completed successfully"); + + } while (0); // Single execution with break-based error handling + + // Cleanup all resources + execute_all_cleanup(); + managed_fclose_all(); + config_cleanup(&config); + logger_cleanup(); + + // Log final status + if (exit_code != ERROR_SUCCESS) { + logger_error(&error_ctx); + } + + return exit_code; +} +``` + +This LLD provides comprehensive implementation details for migrating the backup_logs.sh script to C, including detailed algorithms, data structures, error handling, and performance optimizations specifically designed for embedded RDK systems. diff --git a/backup_logs/docs/backup_logs_migration_HLD.md b/backup_logs/docs/backup_logs_migration_HLD.md new file mode 100644 index 000000000..4e23ee51c --- /dev/null +++ b/backup_logs/docs/backup_logs_migration_HLD.md @@ -0,0 +1,634 @@ +# High-Level Design: backup_logs.sh Migration to C + +## 1. Overview + +### 1.1 Purpose +This document outlines the High-Level Design (HLD) for migrating the existing `backup_logs.sh` shell script to a C implementation. The migration aims to improve performance, reduce memory footprint, and enhance reliability for embedded RDK systems. + +### 1.2 Scope +- Migration of all functionality from `backup_logs.sh` to C code +- Support for both HDD-enabled and HDD-disabled devices +- Maintain compatibility with existing systemd integration +- Preserve log backup and rotation functionality + +### 1.3 Constraints +- Target embedded systems with limited memory (few KBs to few MBs) +- CPU resources are constrained with low clock speeds +- Must be platform-neutral and portable across multiple architectures +- Minimize dynamic memory allocation +- Avoid floating-point arithmetic where possible +- Thread-safe implementation required + +## 2. System Architecture + +### 2.1 Architecture Overview +The C implementation will follow a modular design with the following key components: + +``` +backup_logs (main executable) +├── Configuration Manager +├── Directory Manager +├── Log Backup Engine +├── File Operations Manager +├── Disk Threshold Monitor +├── System Integration Module +└── Error Handler & Logger +``` + +### 2.2 Component Description + +#### 2.2.1 Configuration Manager +- **Purpose**: Load and parse system configuration files +- **Responsibilities**: + - Parse `/etc/include.properties` + - Parse `/etc/device.properties` + - Parse `/etc/env_setup.sh` if available + - Parse `/etc/special_files.properties` for special files handling + - Validate configuration parameters + - Provide configuration data to other modules + - Load and manage special files configuration for `/tmp` and `/etc` operations + +#### 2.2.2 Directory Manager +- **Purpose**: Handle directory creation and validation +- **Responsibilities**: + - Create log workspace directories + - Validate directory permissions + - Manage directory path resolution + - Handle directory cleanup operations + +#### 2.2.3 Log Backup Engine +- **Purpose**: Core backup logic implementation +- **Responsibilities**: + - Implement HDD-enabled device backup strategy + - Implement HDD-disabled device backup strategy with rotation + - Handle log file identification and filtering + - Execute backup operations based on device type + +#### 2.2.4 File Operations Manager +- **Purpose**: Low-level file operations +- **Responsibilities**: + - File moving and copying operations + - File existence checking + - Pattern-based file finding + - Timestamp generation and management + +#### 2.2.5 Disk Threshold Monitor +- **Purpose**: Monitor disk usage and trigger cleanup +- **Responsibilities**: + - Check disk usage percentages + - Trigger cleanup scripts when thresholds exceed + - Integration with existing disk_threshold_check.sh + +#### 2.2.6 System Integration Module +- **Purpose**: System-level integrations +- **Responsibilities**: + - Systemd notification handling + - Integration with external scripts + - Process status reporting + +#### 2.2.7 Error Handler & Logger +- **Purpose**: Centralized error handling and logging +- **Responsibilities**: + - Structured error reporting + - Log message formatting with timestamps + - Error code standardization + +## 3. Data Structures + +### 3.1 Core Data Structures + +```c +typedef struct { + char log_path[PATH_MAX]; + char prev_log_path[PATH_MAX]; + char prev_log_backup_path[PATH_MAX]; + char persistent_path[PATH_MAX]; + bool hdd_enabled; +} backup_config_t; + +typedef enum { + BACKUP_OP_MOVE, + BACKUP_OP_COPY, + BACKUP_OP_DELETE +} backup_operation_type_t; + +typedef struct { + char source_path[PATH_MAX]; + char dest_path[PATH_MAX]; + backup_operation_type_t operation; + char source_extension[32]; + char dest_extension[32]; +} backup_operation_t; +typedef struct { + int error_code; + char error_message[256]; + const char* function_name; + int line_number; +} error_info_t; +``` + +### 3.3 Special Files Configuration + +#### 3.3.1 Special Files Data Structure +```c +typedef enum { + SPECIAL_FILE_COPY = 0, // cp operation + SPECIAL_FILE_MOVE = 1 // mv operation +} special_file_operation_t; + +typedef struct { + char source_path[PATH_MAX]; + char destination_path[PATH_MAX]; + special_file_operation_t operation; + char conditional_check[64]; // Optional condition variable name +} special_file_entry_t; + +typedef struct { + special_file_entry_t entries[MAX_SPECIAL_FILES]; + size_t count; + bool config_loaded; +} special_files_config_t; + +#define MAX_SPECIAL_FILES 32 // Maximum number of special files to handle +``` + +### 3.2 Memory Management Strategy +- Use fixed-size buffers to avoid dynamic allocation +- Implement memory pools for temporary operations +- Stack-based allocation for small, short-lived data +- Pre-allocated arrays for file lists and paths + +#### 3.3.2 Special Files Configuration Format +The special files configuration follows a simple one-filename-per-line format for embedded system efficiency: +```properties +# Special Files Configuration for backup_logs +# Format: One source file path per line +# Comments start with # and empty lines are ignored +# +# Operation determination is handled by the implementation: +# - Files in /tmp are typically moved (mv operation) +# - Configuration and version files are typically copied (cp operation) +# - Destination is automatically determined based on source filename + +# Files from /tmp directory (will be moved) +/tmp/disk_cleanup.log +/tmp/mount_log.txt +/tmp/mount-ta_log.txt + +# Files from /etc directory (will be copied) +/etc/skyversion.txt +/etc/rippleversion.txt + +# Version file from root (will be copied) +/version.txt +``` + +## 3.4 Special Files Configuration Management + +### 3.4.1 Configuration Loading +The special files configuration is loaded from `/etc/special_files.properties` using a simple line-based parser that: +- Supports one filename per line format with comments (lines starting with #) +- Automatically determines operation type based on source file location +- Automatically determines destination filename from source pathname +- Uses LOG_PATH from backup configuration for destination directory +- Provides error reporting for missing or invalid files + +### 3.4.2 Configuration Processing +- Files in `/tmp/` directory are moved (mv operation) to preserve space +- Configuration and version files are copied (cp operation) to preserve originals +- Destination directory is automatically set to the configured LOG_PATH +- Destination filename is extracted from the source file path +- Log warnings for entries with non-existent source files but continue processing + +## 4. Module Interfaces + +### 4.1 Configuration Manager Interface +```c +int config_load(backup_config_t* config); +int config_validate(const backup_config_t* config); +const char* config_get_log_path(void); +bool config_is_hdd_enabled(void); + +// Special files configuration interface +int special_files_load_config(special_files_config_t* config, const char* config_file); +int special_files_validate_entry(const special_file_entry_t* entry); +int special_files_execute_entry(const special_file_entry_t* entry, + const backup_config_t* backup_config); +int special_files_execute_all(const special_files_config_t* config, + const backup_config_t* backup_config); +void special_files_cleanup(void); +``` + +### 4.2 Directory Manager Interface +```c +// Implemented using RDK system utilities +int createDir(char* path); // Create directory if not exists +int emptyFolder(char* path); // Clean directory contents +int filePresentCheck(const char* path); // Check if file/directory exists +int removeFile(const char* path); // Remove file or directory +``` + +### 4.3 Log Backup Engine Interface +```c +int backup_execute_hdd_enabled_strategy(const backup_config_t* config); +int backup_execute_hdd_disabled_strategy(const backup_config_t* config); +int backup_execute_common_operations(const backup_config_t* config); +int move_log_files_by_pattern(const char* source_dir, const char* dest_dir); +``` + +### 4.4 File Operations Interface +```c +// Implemented using RDK system utilities +int copyFiles(const char* source, const char* dest); // Copy file operation +int removeFile(const char* path); // Remove file operation +int filePresentCheck(const char* path); // Check file existence +// Move is implemented as copy + remove sequence +``` + +## 5. Data Flow + +### 5.1 Main Execution Flow (As Implemented) +1. **Initialization Phase** + - Initialize RDK logging system with extended configuration + - Load system configuration from RDK property APIs (`getIncludePropertyData`, `getDevicePropertyData`) + - Create required directories: LOG_PATH, PREV_LOG_PATH, PREV_LOG_BACKUP_PATH + - Clean PREV_LOG_BACKUP_PATH directory + - Create persistent file marker (`logFileBackup`) + - Run disk threshold check script if available + +2. **Pre-Execution Phase** + - Find and remove existing `last_reboot` marker files + - Determine backup strategy based on HDD_ENABLED configuration + +3. **Backup Execution Phase** + - Execute appropriate backup strategy (HDD-enabled or HDD-disabled) + - Move log files using pattern matching (*.txt*, *.log*, bootlog) + - Handle log rotation for HDD-disabled devices + +4. **Common Operations Phase** + - Execute special files operations based on configuration + - Copy system version files (skyversion.txt, rippleversion.txt, version.txt) + - Create new `last_reboot` marker file + - Send systemd notification if available + +5. **Cleanup Phase** + - Cleanup special files manager resources + - Release all allocated resources + - Report final execution status + +### 5.2 Error Handling Flow +- Centralized error handling through error_info_t structure +- Error propagation through return codes +- Logging of all error conditions with context +- Graceful degradation on non-critical failures + +### 5.3 Visual Flow Representation + +The system workflow is represented through detailed diagrams stored in the [diagrams directory](diagrams/backup_logs_flowcharts.md). This includes both Mermaid-format diagrams and text-based alternatives for environments with limited diagram rendering capabilities. + +#### 5.3.1 Main Backup Process Flow +The primary execution flow showing the complete backup process from initialization to completion, including decision points for HDD-enabled vs HDD-disabled devices. Available in both Mermaid and text formats. + +#### 5.3.2 Component Interaction Sequence +A detailed sequence diagram illustrating the interactions between all system components, showing the order of operations and data flow between modules. Includes timing annotations and error paths. + +#### 5.3.3 HDD Disabled Strategy Detail +A specialized flowchart focusing on the complex log rotation logic for HDD-disabled devices, showing the 4-level backup rotation mechanism with all decision points clearly marked. + +#### 5.3.4 Error Handling and Recovery Flow +A comprehensive error handling flowchart showing how different types of errors are categorized, handled, and recovered from in the system. Includes recovery strategies and escalation paths. + +## 6. Key Algorithms + +### 6.1 HDD-Disabled Backup Algorithm +``` +1. Check for existing backup levels (messages.txt, bak1_messages.txt, etc.) +2. If no existing backups: + - Move all logs to PreviousLogs +3. If backup level 1 exists but not level 2: + - Move current logs to PreviousLogs with bak1_ prefix +4. If backup levels 1-2 exist but not level 3: + - Move current logs to PreviousLogs with bak2_ prefix +5. If all backup levels exist: + - Rotate: bak1->root, bak2->bak1, bak3->bak2, current->bak3 +6. Create last_reboot marker file +``` + +### 6.2 HDD-Enabled Backup Algorithm +``` +1. If no messages.txt in PreviousLogs: + - Move all logs to PreviousLogs + - Create last_reboot marker +2. If messages.txt exists: + - Remove existing last_reboot markers + - Create timestamped backup directory + - Move current logs to timestamped directory + - Create last_reboot marker in timestamped directory +``` + +### 6.3 File Pattern Matching Algorithm +- Use POSIX-compliant pattern matching +- Support for wildcard patterns (*.txt, *.log, etc.) +- Efficient directory traversal with depth control +- Filter by file type (regular files vs. symbolic links) + +### 6.5 Special Files Processing Algorithm +``` +1. Load special files configuration from /etc/special_files.properties +2. Parse each line as a single source file path +3. Skip comment lines (starting with #) and empty lines +4. For each special file entry: + a. Check if source file exists (log warning if not found, continue) + b. Determine operation based on source path: + - Files in /tmp/: move operation (copy + delete) + - All other files: copy operation + c. Extract filename from source path for destination + d. Build full destination path using ${LOG_PATH}/filename + e. Verify destination directory exists, create if needed + f. Execute operation (copy or move) based on determination + g. Log operation result and any errors +5. Update operation statistics and cleanup resources +``` + +### 6.6 Special Files Configuration Parser Algorithm +``` +1. Open /etc/special_files.properties file +2. For each line: + a. Skip empty lines and comments (lines starting with #) + b. Trim whitespace and newline characters + c. Store entire line as source_path in special_file_entry_t structure + d. Extract filename from source path for destination_path + e. Set default operation to SPECIAL_FILE_COPY (will be determined at execution) + f. Set conditional_check to empty string +3. Return parsed configuration with entry count or error code +``` + +# Additional Data Structures + +### 3.5 Enhanced Error Handling and Configuration Flags + +```c +/* Complete error code enumeration */ +typedef enum { + BACKUP_SUCCESS = 0, + BACKUP_ERROR_CONFIG = -1, + BACKUP_ERROR_FILESYSTEM = -2, + BACKUP_ERROR_PERMISSIONS = -3, + BACKUP_ERROR_MEMORY = -4, + BACKUP_ERROR_INVALID_PARAM = -5, + BACKUP_ERROR_NOT_FOUND = -6, + BACKUP_ERROR_DISK_FULL = -7, + BACKUP_ERROR_SYSTEM = -8 +} backup_result_t; + +/* Configuration flags for advanced options */ +typedef struct { + bool debug_enabled; + bool force_rotation; + bool skip_disk_check; + bool cleanup_enabled; +} backup_flags_t; + +/* Additional constants */ +#define MAX_ERROR_MESSAGE_LEN 256 +#define MAX_FUNCTION_NAME_LEN 64 +#define MAX_DESCRIPTION_LEN 128 +#define MAX_CONDITIONAL_LEN 64 +#define LOG_BACKUP_LOGS "LOG.RDK.BACKUPLOGS" +``` +The special files processing is integrated into the main backup flow as follows: +- **Phase 1**: Load special files configuration during initialization +- **Phase 2**: Execute special file copy operations before log backup +- **Phase 3**: Execute special file move operations during cleanup phase +- **Phase 4**: Report special files operation status in final logging + +## 7. Threading and Concurrency + +### 7.1 Threading Strategy +- Single-threaded design for simplicity and reliability +- Thread-safe utility functions for potential future extensions +- Use of atomic operations for shared state (if any) + +### 7.2 Synchronization +- File locking for critical operations +- Mutex protection for shared resources (if threading is added later) +- Process-level coordination through lockfiles + +## 8. Performance Considerations + +### 8.1 Memory Optimization +- Fixed-size buffers with compile-time sizing +- Stack allocation preference over heap allocation +- Minimal memory fragmentation through planned allocation patterns +- Efficient string handling with bounded operations + +### 8.2 I/O Optimization +- Batch file operations where possible +- Minimize system calls through buffered operations +- Efficient directory traversal algorithms +- Streaming operations for large files + +## 8.5 RDK Logger Integration + +The implementation includes comprehensive RDK logging framework integration: + +### 8.5.1 Logger Configuration +```c +#ifdef RDK_LOGGER_EXT + rdk_logger_ext_config_t logger_config = { + .pModuleName = "LOG.RDK.BACKUPLOGS", + .loglevel = RDK_LOG_INFO, + .output = RDKLOG_OUTPUT_CONSOLE, + .format = RDKLOG_FORMAT_WITH_TS, + .pFilePolicy = NULL + }; + rdk_logger_ext_init(&logger_config); +#endif +``` + +### 8.5.2 Logger Features +- **Extended Logger Support**: Uses RDK_LOGGER_EXT for enhanced configuration +- **Timestamped Output**: All log messages include timestamps +- **Console Fallback**: Graceful handling when logger initialization fails +- **Debug INI Integration**: Reads configuration from `/etc/debug.ini` +- **Module-Specific Logging**: Uses dedicated "LOG.RDK.BACKUPLOGS" component + +### 8.5.3 Build-time Configuration +```c +#ifdef RDK_LOGGER_EXT +#define RDK_LOGGER_ENABLED +#endif +``` + +The system supports both basic RDK logger and extended RDK logger configurations through compile-time flags. +### 8.6 CPU Optimization +- Avoid expensive operations in loops +- Use bit operations for flags and states +- Minimize string operations and use const strings where possible +- Efficient pattern matching algorithms + +## 9. Integration Points + +### 9.1 RDK System Integration +- **RDK Property System**: Integration with `getIncludePropertyData()` and `getDevicePropertyData()` APIs +- **RDK Logger Framework**: Full support for RDK logging with extended configuration +- **RDK Firmware Utils**: Integration with `fwutils` library for system operations +- **Systemd Integration**: Maintain compatibility with existing service files +- **External Scripts**: Integration with `disk_threshold_check.sh` and other system scripts + +### 9.2 Configuration File Dependencies +- **Include Properties**: `/etc/include.properties` for LOG_PATH and other system paths +- **Device Properties**: `/etc/device.properties` for HDD_ENABLED and device-specific settings +- **Debug Configuration**: `/etc/debug.ini` for RDK logger configuration +- **Special Files**: `/etc/special_files.properties` for configurable file operations (one filename per line format) +- **Environment Setup**: `/etc/env_setup.sh` if available for additional environment variables + +### 9.3 Build System Dependencies +```makefile +# Required libraries and flags +backup_logs_LDADD = -lm -lrdkloggers -lfwutils -lsystemd +backup_logs_CPPFLAGS = -DRDK_LOGGER_EXT +``` + +**Required Dependencies**: +- `librdkloggers` - RDK logging framework +- `libfwutils` - RDK firmware utilities for configuration and system operations +- `libsystemd` - Systemd integration for service notifications +- `libm` - Math library for any mathematical operations + +### 9.4 Backward Compatibility +- Maintain existing directory structure and naming conventions +- Preserve log file formats and timestamps +- Keep existing environment variable usage +- Maintain compatibility with log analysis tools + +## 10. Error Handling Strategy + +### 10.1 Error Categories +- **Fatal Errors**: Configuration failures, permission issues +- **Recoverable Errors**: Individual file operation failures +- **Warnings**: Non-critical issues that don't prevent execution + +### 10.2 Error Reporting +- Structured error codes for programmatic handling +- Human-readable error messages for debugging +- Integration with existing logging infrastructure +- Syslog integration for system-level error reporting + +## 11. Testing Strategy + +### 11.1 Unit Testing +- Test individual modules in isolation +- Mock external dependencies (file system, system calls) +- Comprehensive error condition testing +- Memory leak detection and prevention +- Special files configuration parser testing with various input formats +- Special files operation testing with different file permissions and paths + +### 11.2 Integration Testing +- Test complete backup scenarios +- Verify compatibility with existing system +- Performance benchmarking against shell script +- Multi-platform validation +- Special files configuration end-to-end testing +- Variable substitution testing for different environment setups + +### 11.3 System Testing +- End-to-end functionality verification +- Stress testing with large log volumes +- Resource constraint testing +- Recovery testing after various failure scenarios + +## 12. Deployment Considerations + +### 12.1 Build System +- Integration with existing autotools configuration +- Cross-compilation support for multiple architectures +- Compiler optimization flags for embedded targets +- Static linking considerations for deployment +- **RDK-Specific Build Requirements**: + - RDK Logger framework integration (`-lrdkloggers`) + - RDK Firmware utilities integration (`-lfwutils`) + - Systemd integration (`-lsystemd`) + - Extended logger compile flag (`-DRDK_LOGGER_EXT`) + +### 12.2 Installation +- Backward-compatible installation process +- Service file updates for systemd integration +- Configuration migration support +- Rollback capability + +### 12.3 Monitoring +- Health check mechanisms +- Performance metrics collection +- Resource usage monitoring +- Integration with existing monitoring infrastructure + +## 13. Future Enhancements + +### 13.1 Planned Features +- Configuration hot-reloading capability +- Enhanced compression for archived logs +- Remote log backup capability +- Advanced filtering and retention policies + +### 13.2 Extensibility +- Plugin architecture for custom backup strategies +- Configurable backup policies +- API for external tools integration +- Event-driven architecture support + +## 14. Risk Analysis + +### 14.1 Technical Risks +- **Memory Management**: Risk of memory leaks in embedded environment +- **File System Operations**: Race conditions with concurrent access +- **Configuration Parsing**: Compatibility issues with shell variable expansion +- **Performance**: Potential performance regression compared to shell script + +### 14.2 Mitigation Strategies +- Comprehensive testing with memory analysis tools +- File locking and atomic operations for critical sections +- Robust configuration parsing with validation +- Performance benchmarking and optimization + +## 15. Success Criteria + +### 15.1 Functional Requirements +- ✅ Complete feature parity with existing shell script +- ✅ Support for both HDD-enabled and HDD-disabled devices +- ✅ Proper log rotation and backup functionality +- ✅ Integration with systemd and existing infrastructure + +### 15.2 Non-Functional Requirements +- ✅ Memory usage reduction of at least 20% compared to shell process +- ✅ Startup time improvement of at least 30% +- ✅ CPU usage reduction during backup operations +- ✅ Cross-platform compatibility across target embedded systems + +## 16. Implementation Status + +### 16.1 Completed Features +- ✅ Complete modular architecture with defined components +- ✅ Core backup strategies for HDD-enabled and HDD-disabled devices +- ✅ RDK Logger framework integration with extended configuration +- ✅ Configuration management using RDK property APIs +- ✅ Special files handling with simplified configuration format +- ✅ Error handling with comprehensive error codes +- ✅ Build system integration with RDK dependencies +- ✅ Pattern-based log file identification and movement +- ✅ Systemd integration and external script execution + +### 16.2 Current Limitations / Future Work +- ⚠️ **Command Line Options**: Function definitions exist but CLI parsing not implemented +- ⚠️ **Special Files Format**: Simplified one-filename-per-line format instead of full pipe-separated specification +- ⚠️ **Configuration Validation**: Basic validation implemented, could be enhanced +- ⚠️ **Advanced Configuration**: Variable substitution not implemented in special files + +### 16.3 Architecture Decisions Made +- **Configuration Format**: Chose simplicity over full pipe-separated format for embedded efficiency +- **RDK Integration**: Deep integration with RDK APIs rather than generic POSIX-only approach +- **Error Handling**: Comprehensive error codes with graceful degradation on non-critical failures +- **Logging**: Full RDK logger integration with extended configuration options + +This HLD provides a comprehensive roadmap for migrating the backup_logs.sh script to a robust, efficient C implementation suitable for embedded RDK environments. diff --git a/backup_logs/docs/backup_logs_requirements.md b/backup_logs/docs/backup_logs_requirements.md new file mode 100644 index 000000000..375ef5012 --- /dev/null +++ b/backup_logs/docs/backup_logs_requirements.md @@ -0,0 +1,327 @@ +# Functional Requirements: backup_logs.sh Migration + +## 1. Overview + +This document outlines the detailed functional requirements for migrating the `backup_logs.sh` shell script to a C implementation for embedded RDK systems. + +## 2. Functional Requirements + +### 2.1 Configuration Management (REQ-001) +**Description**: The system must load and parse configuration from RDK property APIs +**Requirements**: +- Use `getIncludePropertyData()` API to retrieve `LOG_PATH` configuration +- Use `getDevicePropertyData()` API to retrieve `HDD_ENABLED` and device-specific settings +- Use `APP_PERSISTENT_PATH` for persistent marker file location +- Construct derived paths: `$LOG_PATH/PreviousLogs`, `$LOG_PATH/PreviousLogs_backup` +- Validate all configuration parameters before proceeding +- Handle missing configuration gracefully with sensible defaults +- Integrate with RDK logging framework for configuration status + +**Input**: RDK property system APIs +**Output**: Structured `backup_config_t` configuration data +**Error Handling**: Log configuration errors using RDK logger and exit with appropriate error code + +### 2.2 Directory Management (REQ-002) +**Description**: Create and manage required log directory structures using RDK utilities +**Requirements**: +- Use `createDir()` function to create `$LOG_PATH` directory if it doesn't exist +- Create `$LOG_PATH/PreviousLogs` directory structure +- Create `$LOG_PATH/PreviousLogs_backup` directory structure +- Use `emptyFolder()` to clean existing backup directory contents before use +- Set appropriate permissions on created directories +- Handle directory creation failures gracefully with proper error reporting +- Use `filePresentCheck()` for directory existence validation + +**Input**: Configuration paths from RDK property system +**Output**: Created directory structures with proper permissions +**Constraints**: Must work with various filesystem types and embedded system constraints + +### 2.3 Disk Threshold Monitoring (REQ-003) +**Description**: Monitor disk usage and trigger cleanup when necessary +**Requirements**: +- Execute disk threshold check if `/lib/rdk/disk_threshold_check.sh` exists +- Pass parameter `0` to the disk check script +- Handle script execution failures without stopping backup process +- Log disk check results for monitoring + +**Input**: Disk check script path +**Output**: Disk status information +**Dependencies**: External `disk_threshold_check.sh` script + +### 2.4 HDD-Disabled Device Backup Strategy (REQ-004) +**Description**: Implement 4-level log rotation for devices without HDD +**Requirements**: +- Support up to 4 backup levels: base, bak1_, bak2_, bak3_ +- Move files based on existing backup level: + - Level 0: Move current logs to PreviousLogs + - Level 1: Move current logs with `bak1_` prefix + - Level 2: Move current logs with `bak2_` prefix + - Level 3: Rotate all levels (bak1→base, bak2→bak1, bak3→bak2, current→bak3) +- Create `last_reboot` marker file after each backup +- Clean current log directory after backup completion + +**Input**: Current log files and existing backup state +**Output**: Rotated backup files with appropriate naming +**File Patterns**: `*.txt*`, `*.log*`, `*.bin*`, `bootlog` files + +### 2.5 HDD-Enabled Device Backup Strategy (REQ-005) +**Description**: Implement timestamped backup for devices with HDD +**Requirements**: +- Check for existing `messages.txt` in PreviousLogs directory +- If no existing backup: Move all logs to PreviousLogs directory +- If backup exists: Create timestamped backup directory (`logbackup-MM-DD-YY-HH-MM-SSAM`) +- Move current logs to timestamped directory +- Remove any existing `last_reboot` markers before creating new one +- Create `last_reboot` marker in appropriate location + +**Input**: Current log files and existing backup state +**Output**: Timestamped backup directories with organized log files +**File Patterns**: `*.txt*`, `*.log*`, `bootlog` files (no `.bin*` files) + +### 2.6 File Operations (REQ-006) +**Description**: Perform reliable file and directory operations +**Requirements**: +- Move files with error handling and validation +- Support pattern-based file finding (find with depth and type constraints) +- Handle both regular files and symbolic links +- Implement atomic file operations where possible +- Validate file operations and report failures +- Support large numbers of files efficiently + +**Input**: Source and destination paths, file patterns +**Output**: Moved/copied files with status reporting +**Constraints**: Must handle filesystem limitations and permissions + +### 2.7 Version File Management (REQ-007) +**Description**: Copy system version information to log directory +**Requirements**: +- Copy `/version.txt` to current log directory +- Copy `/etc/skyversion.txt` to current log directory as `skyversion.txt` +- Copy `/etc/rippleversion.txt` to current log directory as `rippleversion.txt` +- Handle missing version files gracefully (non-fatal errors) +- Preserve file timestamps and permissions where possible + +**Input**: System version files +**Output**: Version files in log directory +**Error Handling**: Log warnings for missing files but continue execution + +### 2.8 Special Log File Handling (REQ-008) - Updated +**Description**: Handle special files using simplified configuration format +**Requirements**: +- Load special files configuration from `/etc/special_files.properties` (one filename per line) +- Support comment lines starting with `#` and empty line skipping +- Automatically determine operation based on source file location: + - Files in `/tmp/`: move operation (preserves space) + - All other files: copy operation (preserves originals) +- Extract destination filename from source path automatically +- Handle atomic file operations using `copyFiles()` and `removeFile()` utilities +- Continue execution if special files are missing (non-fatal) +- Destination directory is automatically set to `$LOG_PATH` + +**Configuration Format**: +``` +# Special files to handle (one per line) +/tmp/disk_cleanup.log +/tmp/mount_log.txt +/tmp/mount-ta_log.txt +/etc/skyversion.txt +/etc/rippleversion.txt +/version.txt +``` + +**Input**: Configuration file with one source path per line +**Output**: Special files moved/copied to log directory +**Timing**: Execute during common operations phase after main backup + +### 2.9 System Integration (REQ-009) +**Description**: Integrate with systemd and system services +**Requirements**: +- Send systemd ready notification upon completion +- Set systemd status message: "Logs Backup Done..!" +- Create persistent marker file at `$PERSISTENT_PATH/logFileBackup` +- Handle systemd notification failures gracefully +- Support operation in non-systemd environments + +**Input**: Completion status +**Output**: System notifications and marker files +**Dependencies**: systemd-notify command availability + +### 2.10 Logging and Monitoring (REQ-010) - Updated +**Description**: Provide comprehensive logging using RDK Logger framework +**Requirements**: +- Initialize RDK Logger with extended configuration if available +- Use `LOG.RDK.BACKUPLOGS` component name for all log messages +- Support different log levels (RDK_LOG_INFO, RDK_LOG_WARN, RDK_LOG_ERROR, RDK_LOG_DEBUG) +- Read logger configuration from `/etc/debug.ini` +- Include timestamped output format when extended logger is enabled +- Fallback to console output if RDK logger initialization fails +- Use `RDK_LOG()` macro for structured logging throughout application +- Log all major operations with appropriate detail level + +**RDK Logger Configuration**: +```c +rdk_logger_ext_config_t logger_config = { + .pModuleName = "LOG.RDK.BACKUPLOGS", + .loglevel = RDK_LOG_INFO, + .output = RDKLOG_OUTPUT_CONSOLE, + .format = RDKLOG_FORMAT_WITH_TS, + .pFilePolicy = NULL +}; +``` + +**Input**: Operation status and error conditions +**Output**: Structured log messages with RDK-compatible format +**Format**: Compatible with RDK logging standards and systemd journal + +## 3. Non-Functional Requirements + +### 3.1 Performance Requirements (NFR-001) +- Memory usage must be ≤ 512KB peak during operation +- Startup time must be ≤ 2 seconds on target hardware +- File operations must complete within 30 seconds for typical log volumes +- CPU usage should not exceed 10% during backup operations + +### 3.2 Reliability Requirements (NFR-002) +- System must handle unexpected shutdowns gracefully +- Backup operations must be atomic (complete or rollback) +- Must recover from partial backup states on restart +- Handle filesystem full conditions without data loss + +### 3.3 Portability Requirements (NFR-003) +- Support ARM, MIPS, and x86 architectures +- Compatible with various embedded Linux distributions +- Work with different filesystem types (ext4, JFFS2, UBIFS) +- Support cross-compilation toolchains + +### 3.4 Security Requirements (NFR-004) +- Validate all file paths to prevent directory traversal +- Handle file permissions correctly without privilege escalation +- Sanitize all inputs from configuration files +- Protect against symlink attacks during file operations + +## 4. Input/Output Specifications + +### 4.1 Inputs (Updated) +- **RDK Property System**: Configuration via `getIncludePropertyData()` and `getDevicePropertyData()` APIs +- **Log Files**: Files matching patterns `*.txt*`, `*.log*`, `bootlog` +- **Version Files**: `/version.txt`, `/etc/skyversion.txt`, `/etc/rippleversion.txt` +- **Special Files Config**: `/etc/special_files.properties` (one filename per line format) +- **Debug Configuration**: `/etc/debug.ini` for RDK logger setup + +### 4.2 Outputs +- **Backup Directories**: Organized log file backups with appropriate naming +- **Marker Files**: `last_reboot` markers for tracking backup cycles +- **System Notifications**: systemd ready notifications and status messages +- **Log Messages**: Timestamped operation logs for monitoring + +### 4.3 Error Codes (Updated) +- **0**: Success (BACKUP_SUCCESS) - All operations completed successfully +- **-1**: Configuration Error (BACKUP_ERROR_CONFIG) - Invalid or missing configuration +- **-2**: Filesystem Error (BACKUP_ERROR_FILESYSTEM) - Directory creation or file operation failure +- **-3**: Permission Error (BACKUP_ERROR_PERMISSIONS) - Insufficient permissions for required operations +- **-4**: Memory Error (BACKUP_ERROR_MEMORY) - Memory allocation failures +- **-5**: Invalid Parameter Error (BACKUP_ERROR_INVALID_PARAM) - Invalid function parameters +- **-6**: Not Found Error (BACKUP_ERROR_NOT_FOUND) - Required files or directories not found +- **-7**: Disk Full Error (BACKUP_ERROR_DISK_FULL) - Insufficient disk space +- **-8**: System Error (BACKUP_ERROR_SYSTEM) - External script or system call failure + +## 5. Dependencies + +### 5.1 RDK System Dependencies +- **RDK Logger Framework**: `librdkloggers` for comprehensive logging +- **RDK Firmware Utils**: `libfwutils` for configuration management and system operations +- **RDK Property APIs**: `getIncludePropertyData()`, `getDevicePropertyData()` for configuration +- **RDK System Utilities**: `createDir()`, `emptyFolder()`, `filePresentCheck()`, `copyFiles()`, `removeFile()` + +### 5.2 System Dependencies +- POSIX-compliant filesystem +- Standard C library (libc) +- systemd integration (`libsystemd`) for service notifications +- systemd-notify utility (optional) +- Math library (`libm`) for numerical operations + +### 5.3 Build Dependencies +```makefile +# Required libraries and flags +backup_logs_LDADD = -lm -lrdkloggers -lfwutils -lsystemd +backup_logs_CPPFLAGS = -DRDK_LOGGER_EXT +``` + +### 5.4 Configuration Dependencies +- `/etc/debug.ini` for RDK logger configuration +- `/etc/special_files.properties` for special files configuration (optional) +- RDK property system for LOG_PATH and HDD_ENABLED configuration +- Access to `/proc` filesystem for system information + +### 5.5 External Scripts +- `/lib/rdk/disk_threshold_check.sh` - Disk usage monitoring +- Configuration parsing utilities for shell variable format + +### 5.6 File System Requirements +- Write access to log directories +- Sufficient disk space for log rotation (minimum 2x current log size) +- Support for atomic file operations (rename) + +## 6. Constraints + +### 6.1 Timing Constraints +- Must complete within systemd service timeout (typically 90 seconds) +- Backup rotation should complete within 10 seconds for typical volumes +- Configuration loading must complete within 1 second + +### 6.2 Memory Constraints +- Peak memory usage limited to 512KB on embedded systems +- No dynamic memory allocation for file lists exceeding 100MB +- Stack usage limited to 64KB maximum depth + +### 6.3 Storage Constraints +- Must work with log directories up to 1GB in size +- Support up to 10,000 individual log files +- Handle filenames up to 255 characters (filesystem limit) + +## 7. Edge Cases and Error Scenarios + +### 7.1 Configuration Edge Cases +- Missing configuration files +- Malformed shell variable syntax +- Invalid path specifications +- Conflicting configuration values +- Unicode characters in paths + +### 7.2 Filesystem Edge Cases +- Disk full during backup operations +- Permission changes during execution +- Network filesystem disconnections +- Corrupted filesystem states +- Very large individual log files (>100MB) + +### 7.3 System Edge Cases +- System shutdown during backup +- Multiple backup processes running simultaneously +- Clock adjustments affecting timestamps +- Filesystem readonly states +- Missing system utilities + +## 8. Acceptance Criteria + +### 8.1 Functional Acceptance +- [ ] All backup strategies work correctly for both HDD configurations +- [ ] Log rotation maintains proper sequence and naming +- [ ] Version file copying works without data loss +- [ ] System integrations (systemd) function properly +- [ ] Error handling provides useful diagnostic information + +### 8.2 Performance Acceptance +- [ ] Memory usage stays within embedded system constraints +- [ ] Startup and completion times meet target requirements +- [ ] File operations scale appropriately with log volume +- [ ] CPU usage remains reasonable during operation + +### 8.3 Reliability Acceptance +- [ ] Operations complete successfully in normal conditions +- [ ] System handles error conditions gracefully +- [ ] Recovery from partial states works correctly +- [ ] No data loss occurs during operations +- [ ] Cross-platform compatibility verified + +This requirements document provides the foundation for implementing a robust, efficient C replacement for the backup_logs.sh script that meets the needs of embedded RDK systems. diff --git a/backup_logs/docs/diagrams/backup_logs_flowcharts.md b/backup_logs/docs/diagrams/backup_logs_flowcharts.md new file mode 100644 index 000000000..7d605d42f --- /dev/null +++ b/backup_logs/docs/diagrams/backup_logs_flowcharts.md @@ -0,0 +1,522 @@ +# Backup Logs Migration - Flowcharts and Diagrams + +## Text-Based Flowchart Alternatives + +### 1. Main Backup Process Flow (Text Alternative) + +``` +START backup_logs + | + v +Initialize RDK Logger (Extended Config) + | + v +Logger Init Success? --> NO --> Fallback to Console Logging + | | + v YES v +Load Configuration from RDK APIs + | + v +getIncludePropertyData("LOG_PATH") + | + v +getDevicePropertyData("HDD_ENABLED") + | + v +Configuration Valid? --> NO --> Log Error & Exit --> END + | + v YES +Create Log Workspace (createDir) + | + v +Create Previous Log Directories (createDir) + | + v +Clean Backup Directory (emptyFolder) + | + v +Create Persistent Marker File + | + v +Check Disk Threshold (/lib/rdk/disk_threshold_check.sh) + | + v +Remove Existing last_reboot Markers + | + v +HDD Enabled? + | + +-- YES --> Execute HDD Enabled Strategy + | | + | v + | Check for Existing messages.txt + | | + | v + | messages.txt Exists? + | | + | +-- NO --> Move All Logs to Previous --> Create Last Reboot Marker + | | + | +-- YES --> Create Timestamped Directory + | | + | v + | Move Logs to Timestamped Dir + | | + | v + | Create Last Reboot Marker + | + +-- NO --> Execute HDD Disabled Strategy + | + v + Check Backup Levels + | + v + Which Level? + | + +-- Level 0 --> Move to Previous Logs --> Create Last Reboot Marker + | + +-- Level 1 --> Move with bak1_ prefix --> Create Last Reboot Marker + | + +-- Level 2 --> Move with bak2_ prefix --> Create Last Reboot Marker + | + +-- Level 3 --> Rotate All Backup Levels --> Create Last Reboot Marker + +All paths converge to: + | + v +Execute Common Operations + | + v +Load Special Files Config (/etc/special_files.properties) + | + v +Process Special Files (one filename per line) + | + v +Copy Version Files (skyversion.txt, rippleversion.txt, version.txt) + | + v +Send Systemd Notification + | + v +Cleanup Resources + | + v +END +``` + +### 2. HDD Disabled Strategy Detail (Text Alternative) + +``` +START HDD Disabled Strategy + | + v +Remove existing last_reboot marker + | + v +Check for messages.txt in Previous Logs + | + v +messages.txt exists? + | + +-- NO --> Find all *.txt, *.log, *.bin, bootlog files + | | + | v + | Move files from LOG_PATH to PREV_LOG_PATH + | | + | v + | Create last_reboot marker --> END STRATEGY + | + +-- YES --> Check for bak1_messages.txt + | + v + bak1_messages.txt exists? + | + +-- NO --> Move current logs with bak1_ prefix --> Create last_reboot marker --> END STRATEGY + | + +-- YES --> Check for bak2_messages.txt + | + v + bak2_messages.txt exists? + | + +-- NO --> Move current logs with bak2_ prefix --> Create last_reboot marker --> END STRATEGY + | + +-- YES --> Check for bak3_messages.txt + | + v + bak3_messages.txt exists? + | + +-- NO --> Move current logs with bak3_ prefix --> Create last_reboot marker --> END STRATEGY + | + +-- YES --> Start Rotation Process + | + v + Move bak1_ files to root names + | + v + Move bak2_ files to bak1_ names + | + v + Move bak3_ files to bak2_ names + | + v + Move current logs to bak3_ names + | + v + Create last_reboot marker --> END STRATEGY +``` + +### 3. Component Interaction Sequence (Text Alternative) + +``` +Main Process -> Logger: Initialize logging system +Main Process -> Config Manager: Load configuration files +Config Manager -> Config Manager: Parse /etc/include.properties +Config Manager -> Config Manager: Parse /etc/device.properties +Config Manager -> Config Manager: Parse /etc/env_setup.sh +Config Manager --> Main Process: Configuration data + +Main Process -> Directory Manager: Create log workspace directories +Directory Manager -> File Operations: Create directory if not exists +File Operations --> Directory Manager: Directory creation status +Directory Manager --> Main Process: Workspace ready + +Main Process -> Disk Monitor: Check disk threshold +Disk Monitor -> File Operations: Execute disk_threshold_check.sh +File Operations --> Disk Monitor: Threshold check result +Disk Monitor --> Main Process: Disk status + +Main Process -> Backup Engine: Execute backup strategy + +IF HDD Enabled Device: + Backup Engine -> File Operations: Check for existing messages.txt + File Operations --> Backup Engine: File existence result + + IF No existing backup: + Backup Engine -> File Operations: Move all logs to Previous + ELSE IF Existing backup found: + Backup Engine -> Directory Manager: Create timestamped directory + Backup Engine -> File Operations: Move logs to timestamped directory + +ELSE IF HDD Disabled Device: + Backup Engine -> File Operations: Check backup levels + File Operations --> Backup Engine: Current backup level + + IF Level 0-2: + Backup Engine -> File Operations: Move with appropriate prefix + ELSE IF Level 3: + Backup Engine -> File Operations: Rotate all backup levels + +Backup Engine -> File Operations: Create last_reboot marker +File Operations --> Backup Engine: Marker creation status +Backup Engine --> Main Process: Backup operation complete + +Main Process -> File Operations: Clean current log directory +Main Process -> File Operations: Copy version files +Main Process -> File Operations: Handle special log files + +Main Process -> System Integration: Send systemd notification +System Integration --> Main Process: Notification sent + +Main Process -> Logger: Log completion status +Logger --> Main Process: Logging complete +``` + +### 4. Special Files Processing Flow (Actual Implementation) + +``` +START Special Files Processing + | + v +Load /etc/special_files.properties + | + v +File Exists? + | + +-- NO --> Log Warning --> END (Non-fatal) + | + +-- YES --> Read File Line by Line + | + v + For Each Line: + | + v + Skip Comments (#) and Empty Lines + | + v + Extract Source Path (entire line) + | + v + Extract Destination Filename from Source Path + | + v + Determine Operation Based on Source Path: + | + +-- /tmp/* --> Move Operation (copyFiles + remove) + | + +-- Other --> Copy Operation (copyFiles only) + | + v + Check Source File Exists? + | + +-- NO --> Log Warning --> Continue Next File + | + +-- YES --> Build Destination Path (LOG_PATH + filename) + | + v + Execute Operation + | + v + Log Operation Result + | + v + Continue Next File + | + v + Process Complete + | + v + END Special Files Processing +``` + +``` +Function Call + | + v +Operation Successful? + | + +-- YES --> Return Success Code --> End + | + +-- NO --> Capture Error Context + | + v + Determine Error Severity + | + v + Error Type? + | + +-- Fatal --> Log Fatal Error + | | + | v + | Cleanup Resources + | | + | v + | Send Emergency Notification + | | + | v + | Exit Process --> End + | + +-- Recoverable --> Log Warning + | | + | v + | Log Error Details + | | + | v + | Attempt Recovery + | | + | v + | Recovery Successful? + | | + | +-- YES --> Continue Operation --> Return Success Code --> End + | | + | +-- NO --> Escalate to Critical + | | + | v + | Log Critical Error (see below) + | + +-- Critical System --> Log Critical Error + | + v + Log to Syslog + | + v + Notify System Monitor + | + v + Attempt Graceful Shutdown + | + v + Return Error Code --> End +``` + +## Mermaid Diagram Sources + +### Main Backup Process Flow (Mermaid) +```mermaid +flowchart TD + A[Start backup_logs] --> B[Initialize Logging] + B --> C[Load Configuration] + C --> D{Configuration Valid?} + D -->|No| E[Log Error & Exit] + D -->|Yes| F[Create Log Workspace] + F --> G[Create Previous Log Directories] + G --> H[Check Disk Threshold] + + H --> I{HDD Enabled?} + I -->|Yes| J[Execute HDD Enabled Strategy] + I -->|No| K[Execute HDD Disabled Strategy] + + J --> L[Check for Existing Backup] + L --> M{Backup Exists?} + M -->|No| N[Move All Logs to Previous] + M -->|Yes| O[Create Timestamped Directory] + O --> P[Move Logs to Timestamped Dir] + P --> Q[Create Last Reboot Marker] + + K --> R[Check Backup Levels] + R --> S{Which Level?} + S -->|Level 0| T[Move to Previous Logs] + S -->|Level 1| U[Move with bak1_ prefix] + S -->|Level 2| V[Move with bak2_ prefix] + S -->|Level 3| W[Rotate All Backup Levels] + + T --> X[Create Last Reboot Marker] + U --> X + V --> X + W --> X + N --> Q + Q --> X + + X --> Y[Clean Current Log Directory] + Y --> Z[Copy Version Files] + Z --> AA[Handle Special Log Files] + AA --> BB[Send Systemd Notification] + BB --> CC[End] + + E --> CC +``` + +### HDD Disabled Strategy Detail (Mermaid) +```mermaid +flowchart TD + A[Start HDD Disabled Strategy] --> B[Remove existing last_bootfile] + B --> C[Check for messages.txt in Previous Logs] + + C --> D{messages.txt exists?} + D -->|No| E[Find all *.txt, *.log, *.bin, bootlog files] + E --> F[Move files from LOG_PATH to PREV_LOG_PATH] + F --> G[Create last_reboot marker] + G --> Z[End Strategy] + + D -->|Yes| H[Check for bak1_messages.txt] + H --> I{bak1_messages.txt exists?} + I -->|No| J[Move current logs with bak1_ prefix] + J --> G + + I -->|Yes| K[Check for bak2_messages.txt] + K --> L{bak2_messages.txt exists?} + L -->|No| M[Move current logs with bak2_ prefix] + M --> G + + L -->|Yes| N[Check for bak3_messages.txt] + N --> O{bak3_messages.txt exists?} + O -->|No| P[Move current logs with bak3_ prefix] + P --> G + + O -->|Yes| Q[Start Rotation Process] + Q --> R[Move bak1_ files to root names] + R --> S[Move bak2_ files to bak1_ names] + S --> T[Move bak3_ files to bak2_ names] + T --> U[Move current logs to bak3_ names] + U --> G +``` + +### Component Interaction Sequence (Mermaid) +```mermaid +sequenceDiagram + participant Main as Main Process + participant Config as Configuration Manager + participant Dir as Directory Manager + participant Backup as Log Backup Engine + participant FileOps as File Operations Manager + participant Disk as Disk Threshold Monitor + participant SysInt as System Integration Module + participant Logger as Error Handler & Logger + + Main->>Logger: Initialize logging system + Main->>Config: Load configuration files + Config->>Config: Parse /etc/include.properties + Config->>Config: Parse /etc/device.properties + Config->>Config: Parse /etc/env_setup.sh + Config-->>Main: Configuration data + + Main->>Dir: Create log workspace directories + Dir->>FileOps: Create directory if not exists + FileOps-->>Dir: Directory creation status + Dir-->>Main: Workspace ready + + Main->>Disk: Check disk threshold + Disk->>FileOps: Execute disk_threshold_check.sh + FileOps-->>Disk: Threshold check result + Disk-->>Main: Disk status + + Main->>Backup: Execute backup strategy + + alt HDD Enabled Device + Backup->>FileOps: Check for existing messages.txt + FileOps-->>Backup: File existence result + alt No existing backup + Backup->>FileOps: Move all logs to Previous + else Existing backup found + Backup->>Dir: Create timestamped directory + Backup->>FileOps: Move logs to timestamped directory + end + else HDD Disabled Device + Backup->>FileOps: Check backup levels + FileOps-->>Backup: Current backup level + alt Level 0-2 + Backup->>FileOps: Move with appropriate prefix + else Level 3 + Backup->>FileOps: Rotate all backup levels + end + end + + Backup->>FileOps: Create last_reboot marker + FileOps-->>Backup: Marker creation status + Backup-->>Main: Backup operation complete + + Main->>FileOps: Clean current log directory + Main->>FileOps: Copy version files + Main->>FileOps: Handle special log files + + Main->>SysInt: Send systemd notification + SysInt-->>Main: Notification sent + + Main->>Logger: Log completion status + Logger-->>Main: Logging complete +``` + +### Error Handling and Recovery Flow (Mermaid) +```mermaid +flowchart TD + A[Function Call] --> B{Operation Successful?} + B -->|Yes| C[Return Success Code] + B -->|No| D[Capture Error Context] + + D --> E[Determine Error Severity] + E --> F{Error Type?} + + F -->|Fatal| G[Log Fatal Error] + F -->|Recoverable| H[Log Warning] + F -->|Critical System| I[Log Critical Error] + + G --> J[Cleanup Resources] + J --> K[Send Emergency Notification] + K --> L[Exit Process] + + H --> M[Log Error Details] + M --> N[Attempt Recovery] + N --> O{Recovery Successful?} + O -->|Yes| P[Continue Operation] + O -->|No| Q[Escalate to Critical] + Q --> I + + I --> R[Log to Syslog] + R --> S[Notify System Monitor] + S --> T[Attempt Graceful Shutdown] + T --> U[Return Error Code] + + P --> C + C --> V[End] + L --> V + U --> V +``` From 6d67d23d8322d8f97d119f6a72708009434a1822 Mon Sep 17 00:00:00 2001 From: madhubabutt <114217841+madhubabutt@users.noreply.github.com> Date: Wed, 18 Mar 2026 01:12:33 +0530 Subject: [PATCH 2/4] RDK-60634 [dcm-agent] RDK Coverity Defect Resolution for Device Management (#81) Co-authored-by: mtirum011 --- dcm_parseconf.c | 75 ++++++++++++++++--- dcm_rbus.c | 13 +++- dcm_schedjob.c | 46 +++++++++++- dcm_utils.c | 15 +++- uploadstblogs/src/archive_manager.c | 32 +++++++- uploadstblogs/src/file_operations.c | 1 + uploadstblogs/src/path_handler.c | 24 +++++- uploadstblogs/src/strategies.c | 4 +- uploadstblogs/src/strategy_selector.c | 1 + .../unittest/mocks/mock_file_operations.cpp | 8 ++ .../unittest/mocks/mock_file_operations.h | 2 + 11 files changed, 192 insertions(+), 29 deletions(-) diff --git a/dcm_parseconf.c b/dcm_parseconf.c index 75f736f34..a2adbd5b4 100755 --- a/dcm_parseconf.c +++ b/dcm_parseconf.c @@ -30,6 +30,7 @@ #include #include #include +#include #include #include "dcm_types.h" @@ -67,6 +68,10 @@ static INT32 dcmSettingGetValueFromFile(INT8 *buf, INT8 *file_path, buf[strcspn( buf, "\n" )] = 0; buf[strcspn( buf, "," )] = 0; tempStr = strstr( buf, delim ); + if(tempStr == NULL) { + DCMError("Delimiter '%s' not found in buffer\n", delim); + continue; + } tempStr++; if(tempStr[0] == '\"') @@ -247,6 +252,7 @@ static INT32 dcmSettingStoreTempConf(INT8 *pConffile, INT8 *pTempConf, INT8 *pOp INT32 i = 0; INT32 ret = DCM_SUCCESS; FILE *fp_out_opt = NULL; + size_t bytes_read = 0; FILE *fp_in = fopen(pConffile, "r"); if (fp_in == NULL) { @@ -262,14 +268,20 @@ static INT32 dcmSettingStoreTempConf(INT8 *pConffile, INT8 *pTempConf, INT8 *pOp } fp_out_opt = fopen(pOptConf, "w"); - if (fp_out == NULL) { + if (fp_out_opt == NULL) { ret = DCM_FAILURE; DCMError("Unable to open out file: %s\n", pOptConf); goto exit2; } fseek(fp_in, 0, SEEK_END); + errno = 0; file_len = ftell(fp_in); + if (file_len < 0) { + ret = DCM_FAILURE; + DCMError("Failed to get file size using ftell(): errno=%d (%s)\n", errno, strerror(errno)); + goto exit3; + } fseek(fp_in, 0, SEEK_SET); buff = calloc(file_len+1, 1); @@ -279,7 +291,15 @@ static INT32 dcmSettingStoreTempConf(INT8 *pConffile, INT8 *pTempConf, INT8 *pOp goto exit3; } - fread(buff, 1, file_len, fp_in); + + errno = 0; + bytes_read = fread(buff, 1, file_len, fp_in); + if (bytes_read != (size_t)file_len) { + ret = DCM_FAILURE; + DCMError("Failed to read the entire file. Expected %d bytes, got %zu bytes (errno=%d, %s, ferror=%d, feof=%d)\n", + file_len, bytes_read, errno, strerror(errno), ferror(fp_in), feof(fp_in)); + goto exit4; + } pJson = cJSON_Parse(buff); if (pJson == NULL) { @@ -356,32 +376,67 @@ static INT32 dcmSettingStoreTempConf(INT8 *pConffile, INT8 *pTempConf, INT8 *pOp fprintf(fp_out_opt, "\"%s\":\"%s\",", tprochitem->string, tprochitem->valuestring); } } - fseek(fp_out, -1, SEEK_CUR); - fseek(fp_out_opt, -1, SEEK_CUR); + errno = 0; + if (fseek(fp_out, -1, SEEK_CUR) != 0) { + ret = DCM_FAILURE; + DCMError("fseek failed on fp_out: errno=%d (%s)\n", errno, strerror(errno)); + goto exit4; + } + errno = 0; + if (fseek(fp_out_opt, -1, SEEK_CUR) != 0) { + ret = DCM_FAILURE; + DCMError("fseek failed on fp_out_opt: errno=%d (%s)\n", errno, strerror(errno)); + goto exit4; + } fprintf(fp_out, "},"); fprintf(fp_out_opt, "},"); } //for (k) - fseek(fp_out, -1, SEEK_CUR); - fseek(fp_out_opt, -1, SEEK_CUR); + errno = 0; + if (fseek(fp_out, -1, SEEK_CUR) != 0) { + ret = DCM_FAILURE; + DCMError("fseek failed on fp_out: errno=%d (%s)\n", errno, strerror(errno)); + goto exit4; + } + errno = 0; + if (fseek(fp_out_opt, -1, SEEK_CUR) != 0) { + ret = DCM_FAILURE; + DCMError("fseek failed on fp_out_opt: errno=%d (%s)\n", errno, strerror(errno)); + goto exit4; + } fprintf(fp_out, "],"); fprintf(fp_out_opt, "],"); } //else if(cJSON_IsArray(titem)) } //for (j) - fseek(fp_out, -1, SEEK_CUR); - fseek(fp_out_opt, -1, SEEK_CUR); + errno = 0; + if (fseek(fp_out, -1, SEEK_CUR) != 0) { + ret = DCM_FAILURE; + DCMError("fseek failed on fp_out: errno=%d (%s)\n", errno, strerror(errno)); + goto exit4; + } + errno = 0; + if (fseek(fp_out_opt, -1, SEEK_CUR) != 0) { + ret = DCM_FAILURE; + DCMError("fseek failed on fp_out_opt: errno=%d (%s)\n", errno, strerror(errno)); + goto exit4; + } fprintf(fp_out, "}\n"); fprintf(fp_out_opt, "}\n"); } //else if (cJSON_IsObject(item)) } //for (i) - cJSON_Delete(pJson); exit4: + if (pJson) { + cJSON_Delete(pJson); + pJson = NULL; + } free(buff); exit3: - fclose(fp_out_opt); + if (fp_out_opt) { + fclose(fp_out_opt); + } exit2: fclose(fp_out); exit1: diff --git a/dcm_rbus.c b/dcm_rbus.c index e487dd457..6953b15af 100644 --- a/dcm_rbus.c +++ b/dcm_rbus.c @@ -89,11 +89,16 @@ static VOID rbusSetConf(rbusHandle_t handle, if(configPath) { const INT8 *filePath = rbusValue_GetString(configPath, NULL); - strcpy(pDCMRbusHandle->confPath, filePath); - DCMInfo("configPath: %s\n", filePath); + if(filePath != NULL) { + strncpy(pDCMRbusHandle->confPath, filePath, DCM_CONF_SIZE - 1); + pDCMRbusHandle->confPath[DCM_CONF_SIZE - 1] = '\0'; + DCMInfo("configPath: %s\n", filePath); + } else { + DCMError("configPath value is NULL or invalid\n"); + } } - DCMInfo("Recieved eventName: %s, Event type: %d, Event Name: %s\n", subscription->eventName, event->type, event->name); + DCMInfo("Received eventName: %s, Event type: %d, Event Name: %s\n", subscription->eventName, event->type, event->name); } @@ -129,7 +134,7 @@ static VOID rbusProcConf(rbusHandle_t handle, return; } - DCMInfo("Recieved eventName: %s, Event type: %d, Event Name: %s\n", subscription->eventName, event->type, event->name); + DCMInfo("Received eventName: %s, Event type: %d, Event Name: %s\n", subscription->eventName, event->type, event->name); pDCMRbusHandle->schedJob = 1; } diff --git a/dcm_schedjob.c b/dcm_schedjob.c index 51347e59b..4501afde0 100644 --- a/dcm_schedjob.c +++ b/dcm_schedjob.c @@ -52,13 +52,32 @@ void* dcmSchedulerThread(void *arg) struct timespec _now; time_t timeOffset, currentTime; - while(!pDCMSched->terminated) { + while(1) { pthread_mutex_lock(&pDCMSched->tMutex); - if(!pDCMSched->startSched) { + // Check termination condition while holding the lock + if(pDCMSched->terminated) { + pthread_mutex_unlock(&pDCMSched->tMutex); + break; + } + + // Wait for scheduling to start - use proper loop for spurious wakeups + while(!pDCMSched->startSched && !pDCMSched->terminated) { n = pthread_cond_wait(&pDCMSched->tCond, &pDCMSched->tMutex); + if(n != 0) { + DCMWarn("%s pthread_cond_wait failed: %d (%s)\n", pDCMSched->name, n, strerror(n)); + pthread_mutex_unlock(&pDCMSched->tMutex); + goto thread_exit; + } } - else { + + // Check termination again after wait + if(pDCMSched->terminated) { + pthread_mutex_unlock(&pDCMSched->tMutex); + break; + } + + if(pDCMSched->startSched) { memset(&_now, 0, sizeof(struct timespec)); clock_gettime(CLOCK_REALTIME, &_now); @@ -67,7 +86,25 @@ void* dcmSchedulerThread(void *arg) timeOffset = dcmCronParseGetNext(&pDCMSched->parseData, currentTime); _now.tv_sec += (timeOffset - currentTime); - n = pthread_cond_timedwait(&pDCMSched->tCond, &pDCMSched->tMutex, &_now); + // Wait with predicate re-check under lock to handle spurious wakeups + while(pDCMSched->startSched && !pDCMSched->terminated) { + n = pthread_cond_timedwait(&pDCMSched->tCond, &pDCMSched->tMutex, &_now); + + if(n == ETIMEDOUT) { + break; + } + + if(n != 0) { + DCMWarn("%s pthread_cond_timedwait failed: %d (%s)\n", pDCMSched->name, n, strerror(n)); + pthread_mutex_unlock(&pDCMSched->tMutex); + goto thread_exit; + } + } + + if(pDCMSched->terminated) { + pthread_mutex_unlock(&pDCMSched->tMutex); + goto thread_exit; + } if(n == ETIMEDOUT) { DCMInfo("Scheduling %s Job handle: %p\n", pDCMSched->name, pDCMSched->pUserData); @@ -87,6 +124,7 @@ void* dcmSchedulerThread(void *arg) } pthread_mutex_unlock(&pDCMSched->tMutex); } +thread_exit: return NULL; } diff --git a/dcm_utils.c b/dcm_utils.c index 45e7665cb..a48e0d183 100644 --- a/dcm_utils.c +++ b/dcm_utils.c @@ -32,6 +32,7 @@ #include #include #include +#include #include "dcm_types.h" #include "dcm_utils.h" @@ -96,9 +97,9 @@ VOID dcmUtilsCopyCommandOutput (INT8 *cmd, INT8 *out, INT32 len) if (fp) { if(out) { if (fgets (out, len, fp) != NULL) { - size_t len = strlen (out); - if ((len > 0) && (out[len - 1] == '\n')) - out[len - 1] = 0; + size_t str_len = strlen (out); + if ((str_len > 0) && (out[str_len - 1] == '\n')) + out[str_len - 1] = 0; } } pclose (fp); @@ -185,7 +186,13 @@ VOID dcmUtilsRemovePIDfile() fp = fopen(DCM_PID_FILE, "r"); if(fp) { fclose(fp); - remove(DCM_PID_FILE); + errno = 0; + if (remove(DCM_PID_FILE) != 0) { + if (errno != ENOENT) { + DCMError("Failed to remove PID file: %s errno=%d (%s)\n", + DCM_PID_FILE, errno, strerror(errno)); + } + } } } diff --git a/uploadstblogs/src/archive_manager.c b/uploadstblogs/src/archive_manager.c index b6357c8e3..28f736bf4 100755 --- a/uploadstblogs/src/archive_manager.c +++ b/uploadstblogs/src/archive_manager.c @@ -700,7 +700,7 @@ static int create_archive_with_options(RuntimeContext* ctx, SessionState* sessio RDK_LOG(RDK_LOG_DEBUG, LOG_UPLOADSTB, "[%s:%d] Creating archive with MAC='%s', prefix='%s'\n", __FUNCTION__, __LINE__, - ctx->mac_address ? ctx->mac_address : "(NULL)", + (ctx->mac_address[0] != '\0') ? ctx->mac_address : "(NULL)", prefix); char archive_filename[MAX_FILENAME_LENGTH]; @@ -736,10 +736,36 @@ static int create_archive_with_options(RuntimeContext* ctx, SessionState* sessio // Write two 512-byte blocks of zeros (TAR EOF marker) char eof_blocks[TAR_BLOCK_SIZE * 2]; memset(eof_blocks, 0, sizeof(eof_blocks)); - gzwrite(gz, eof_blocks, sizeof(eof_blocks)); + if (gzwrite(gz, eof_blocks, sizeof(eof_blocks)) != sizeof(eof_blocks)) { + int zerr = Z_OK; + const char* zmsg = gzerror(gz, &zerr); + RDK_LOG(RDK_LOG_ERROR, LOG_UPLOADSTB, + "[%s:%d] gzwrite failed to write EOF blocks (zerr=%d, msg=%s)\n", + __FUNCTION__, __LINE__, zerr, zmsg ? zmsg : "(null)"); + ret = -1; + } // Close gzip file - gzclose(gz); + int gzclose_ret = gzclose(gz); + if (gzclose_ret != Z_OK) { + const char* zmsg = zError(gzclose_ret); + RDK_LOG(RDK_LOG_ERROR, LOG_UPLOADSTB, + "[%s:%d] gzclose failed (zret=%d, msg=%s)\n", + __FUNCTION__, __LINE__, gzclose_ret, zmsg ? zmsg : "(null)"); + ret = -1; + } + + if (ret != 0 && file_exists(archive_path)) { + RDK_LOG(RDK_LOG_WARN, LOG_UPLOADSTB, + "[%s:%d] Removing incomplete archive: %s\n", + __FUNCTION__, __LINE__, archive_path); + errno = 0; + if (!remove_file(archive_path)) { + RDK_LOG(RDK_LOG_ERROR, LOG_UPLOADSTB, + "[%s:%d] Failed to remove incomplete archive: %s (errno=%d, %s)\n", + __FUNCTION__, __LINE__, archive_path, errno, strerror(errno)); + } + } if (ret == 0 && file_exists(archive_path)) { long size = get_archive_size(archive_path); diff --git a/uploadstblogs/src/file_operations.c b/uploadstblogs/src/file_operations.c index 2bd70cf3b..b3eb9cad6 100755 --- a/uploadstblogs/src/file_operations.c +++ b/uploadstblogs/src/file_operations.c @@ -126,6 +126,7 @@ bool create_directory(const char* dirpath) if (createDir(path_copy) != RDK_API_SUCCESS) { RDK_LOG(RDK_LOG_ERROR, LOG_UPLOADSTB, "[%s:%d] Failed to create directory %s\n", __FUNCTION__, __LINE__, path_copy); + // coverity[MISSING_RESTORE : FALSE] Restore is not needed because function returns immediately. return false; } } diff --git a/uploadstblogs/src/path_handler.c b/uploadstblogs/src/path_handler.c index ac81f305f..3172eab76 100755 --- a/uploadstblogs/src/path_handler.c +++ b/uploadstblogs/src/path_handler.c @@ -24,6 +24,7 @@ #include #include +#include #include "path_handler.h" #include "verification.h" #include "md5_utils.h" @@ -505,8 +506,27 @@ static UploadResult perform_s3_put_with_fallback(RuntimeContext* ctx, SessionSta FILE* curl_info = fopen("/tmp/logupload_curl_info", "r"); if (curl_info) { long http_code = 0; - fscanf(curl_info, "%ld", &http_code); - session->http_code = (int)http_code; + int scan_result; + errno = 0; + scan_result = fscanf(curl_info, "%ld", &http_code); + if (scan_result == 1) { + session->http_code = (int)http_code; + } else { + if (ferror(curl_info)) { + RDK_LOG(RDK_LOG_ERROR, LOG_UPLOADSTB, + "[%s:%d] Failed to read HTTP code from curl info file due to I/O error errno=%d (%s)\n", + __FUNCTION__, __LINE__, errno, strerror(errno)); + } else if (feof(curl_info)) { + RDK_LOG(RDK_LOG_ERROR, LOG_UPLOADSTB, + "[%s:%d] Failed to read HTTP code from curl info file: unexpected EOF\n", + __FUNCTION__, __LINE__); + } else { + RDK_LOG(RDK_LOG_ERROR, LOG_UPLOADSTB, + "[%s:%d] Failed to parse HTTP code from curl info file (scan_result=%d)\n", + __FUNCTION__, __LINE__, scan_result); + } + session->http_code = -1; + } fclose(curl_info); } session->curl_code = s3_result; diff --git a/uploadstblogs/src/strategies.c b/uploadstblogs/src/strategies.c index ca7e84f7c..1112d7d75 100755 --- a/uploadstblogs/src/strategies.c +++ b/uploadstblogs/src/strategies.c @@ -475,8 +475,8 @@ static int ondemand_archive(RuntimeContext* ctx, SessionState* session) "[%s:%d] Context before create_archive: ctx=%p, MAC='%s', device_type='%s'\n", __FUNCTION__, __LINE__, (void*)ctx, - ctx && ctx->mac_address ? ctx->mac_address : "(NULL/INVALID)", - (ctx && strlen(ctx->device_type) > 0) ? ctx->device_type : "(empty/NULL)"); + (ctx && ctx->mac_address[0] != '\0') ? ctx->mac_address : "(NULL/INVALID)", + (ctx && ctx->device_type[0] != '\0') ? ctx->device_type : "(empty/NULL)"); // Create archive from temp directory (NO timestamp modification) int ret = create_archive(ctx, session, ONDEMAND_TEMP_DIR); diff --git a/uploadstblogs/src/strategy_selector.c b/uploadstblogs/src/strategy_selector.c index b0a54077f..0b983838f 100755 --- a/uploadstblogs/src/strategy_selector.c +++ b/uploadstblogs/src/strategy_selector.c @@ -218,6 +218,7 @@ void decide_paths(const RuntimeContext* ctx, SessionState* session) // Direct blocked: CodeBig primary, no fallback else if (direct_blocked && !codebig_blocked) { session->primary = PATH_CODEBIG; + // coverity[copy_paste_error : FALSE] Intentional fallback is PATH_NONE when direct is blocked and CodeBig is primary. session->fallback = PATH_NONE; RDK_LOG(RDK_LOG_INFO, LOG_UPLOADSTB, "[%s:%d] Paths: Primary=CODEBIG, Fallback=NONE (direct blocked)\n", diff --git a/uploadstblogs/unittest/mocks/mock_file_operations.cpp b/uploadstblogs/unittest/mocks/mock_file_operations.cpp index 4fd8eb098..83abb8aa1 100755 --- a/uploadstblogs/unittest/mocks/mock_file_operations.cpp +++ b/uploadstblogs/unittest/mocks/mock_file_operations.cpp @@ -64,6 +64,14 @@ bool copy_file(const char* src, const char* dest) { return true; } +bool remove_file(const char* filepath) { + if (g_mockFileOperations) { + return g_mockFileOperations->remove_file(filepath); + } + if (!filepath) return false; + return true; +} + void emit_system_validation_event(const char* component, bool success) { if (g_mockFileOperations) { g_mockFileOperations->emit_system_validation_event(component, success); diff --git a/uploadstblogs/unittest/mocks/mock_file_operations.h b/uploadstblogs/unittest/mocks/mock_file_operations.h index eedc4dddd..c30f2cc7e 100755 --- a/uploadstblogs/unittest/mocks/mock_file_operations.h +++ b/uploadstblogs/unittest/mocks/mock_file_operations.h @@ -31,6 +31,7 @@ bool file_exists(const char* filepath); bool dir_exists(const char* dirpath); bool create_directory(const char* dirpath); bool copy_file(const char* src, const char* dest); +bool remove_file(const char* filepath); void emit_system_validation_event(const char* component, bool success); void emit_folder_missing_error(void); int v_secure_system(const char* command, ...); @@ -47,6 +48,7 @@ class MockFileOperations { MOCK_METHOD1(dir_exists, bool(const char* dirpath)); MOCK_METHOD1(create_directory, bool(const char* dirpath)); MOCK_METHOD2(copy_file, bool(const char* src, const char* dest)); + MOCK_METHOD1(remove_file, bool(const char* filepath)); MOCK_METHOD2(emit_system_validation_event, void(const char* component, bool success)); MOCK_METHOD0(emit_folder_missing_error, void(void)); MOCK_METHOD1(v_secure_system, int(const char* command)); From 10f09d27d7200e5f8474303bbc7689f6bf8eeefa Mon Sep 17 00:00:00 2001 From: nhanas001c Date: Wed, 18 Mar 2026 19:21:49 +0000 Subject: [PATCH 3/4] tr69hostif 2.0.4 release changelog updates --- CHANGELOG.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b2b8baff5..e7bb0fee1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,9 +4,20 @@ All notable changes to this project will be documented in this file. Dates are d Generated by [`auto-changelog`](https://github.com/CookPete/auto-changelog). +#### [2.0.4](https://github.com/rdkcentral/dcm-agent/compare/2.0.3...2.0.4) + +- RDK-60634 [dcm-agent] RDK Coverity Defect Resolution for Device Management [`#81`](https://github.com/rdkcentral/dcm-agent/pull/81) +- RDK-61009 : [RDKE] Port Log Backup Scripts to Source code [`#95`](https://github.com/rdkcentral/dcm-agent/pull/95) +- RDK-60497 : Port USB Log Upload Scripts to Source code [`#91`](https://github.com/rdkcentral/dcm-agent/pull/91) +- RDK-60497 : Port USB Log Upload Scripts to Source code [`#79`](https://github.com/rdkcentral/dcm-agent/pull/79) +- Merge tag '2.0.3' into develop [`86c4755`](https://github.com/rdkcentral/dcm-agent/commit/86c47550324d871f804446459dbb0a30814d5a2a) + #### [2.0.3](https://github.com/rdkcentral/dcm-agent/compare/2.0.2...2.0.3) +> 11 February 2026 + - Update context_manager.c [`#73`](https://github.com/rdkcentral/dcm-agent/pull/73) +- DCM Agent 2.0.3 release changelog updates [`45018b7`](https://github.com/rdkcentral/dcm-agent/commit/45018b7808de12690a91b45447b372ebd4af0b11) #### [2.0.2](https://github.com/rdkcentral/dcm-agent/compare/2.0.1...2.0.2) From 0c9f034da844be6d2b95d17f7e6d20efbc7acd07 Mon Sep 17 00:00:00 2001 From: Shibu Kakkoth Vayalambron Date: Tue, 24 Mar 2026 12:47:18 -0700 Subject: [PATCH 4/4] Add tools and skills for agentic development (#102) * Add tools and skills for agentic development * Update .github/skills/triage-logs/SKILL.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update .github/skills/quality-checker/SKILL.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update .github/skills/technical-documentation-writer/SKILL.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update .github/skills/platform-portability-checker/SKILL.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update .github/skills/technical-documentation-writer/SKILL.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update .github/skills/quality-checker/SKILL.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .github/agents/embedded-programmer.agent.md | 178 +++++ .github/agents/l2-test-runner.agent.md | 283 +++++++ .../legacy-refactor-specialist.agent.md | 263 +++++++ .../instructions/build-system.instructions.md | 140 ++++ .../instructions/c-embedded.instructions.md | 693 +++++++++++++++++ .../instructions/cpp-testing.instructions.md | 182 +++++ .../shell-scripts.instructions.md | 179 +++++ .../skills/memory-safety-analyzer/SKILL.md | 227 ++++++ .../platform-portability-checker/SKILL.md | 318 ++++++++ .github/skills/quality-checker/README.md | 72 ++ .github/skills/quality-checker/SKILL.md | 329 ++++++++ .../technical-documentation-writer/SKILL.md | 712 ++++++++++++++++++ .../skills/thread-safety-analyzer/SKILL.md | 436 +++++++++++ .github/skills/triage-logs/SKILL.md | 398 ++++++++++ 14 files changed, 4410 insertions(+) create mode 100644 .github/agents/embedded-programmer.agent.md create mode 100644 .github/agents/l2-test-runner.agent.md create mode 100644 .github/agents/legacy-refactor-specialist.agent.md create mode 100644 .github/instructions/build-system.instructions.md create mode 100644 .github/instructions/c-embedded.instructions.md create mode 100644 .github/instructions/cpp-testing.instructions.md create mode 100644 .github/instructions/shell-scripts.instructions.md create mode 100644 .github/skills/memory-safety-analyzer/SKILL.md create mode 100644 .github/skills/platform-portability-checker/SKILL.md create mode 100644 .github/skills/quality-checker/README.md create mode 100644 .github/skills/quality-checker/SKILL.md create mode 100644 .github/skills/technical-documentation-writer/SKILL.md create mode 100644 .github/skills/thread-safety-analyzer/SKILL.md create mode 100644 .github/skills/triage-logs/SKILL.md diff --git a/.github/agents/embedded-programmer.agent.md b/.github/agents/embedded-programmer.agent.md new file mode 100644 index 000000000..8f7ad9724 --- /dev/null +++ b/.github/agents/embedded-programmer.agent.md @@ -0,0 +1,178 @@ +--- +name: 'Embedded Programming Expert' +description: 'Expert in embedded C development with focus on resource constraints, memory safety, and platform independence for RDK Device Management systems including dcm-agent, log upload, and log backup functionality' +tools: ['codebase', 'search', 'edit', 'runCommands', 'problems', 'web'] +--- + +# Embedded C Development Expert + +You are an expert embedded systems C developer specializing in resource-constrained environments. You have deep knowledge of: + +- Memory management without garbage collection +- Platform-independent C programming +- Real-time and embedded systems constraints +- RDK (Reference Design Kit) architecture +- Device Configuration Management (DCM) for RDK devices +- Log upload and backup systems for embedded devices +- RBUS messaging integration for RDK components + +## Your Expertise + +### Memory Management +- RAII patterns in C using cleanup functions +- Memory pools and custom allocators +- Fragmentation prevention strategies +- Stack vs heap tradeoffs +- Valgrind and memory leak detection + +### Thread Safety and Concurrency +- Lightweight synchronization primitives (atomic operations, simple mutexes) +- Deadlock prevention (lock ordering, timeouts) +- Minimal thread memory configuration (pthread attributes) +- Lock-free patterns for embedded systems +- Thread pool design to prevent fragmentation +- Race condition detection and prevention + +### Resource Optimization +- Minimal CPU usage patterns +- Code size reduction techniques +- Static memory allocation strategies +- Efficient data structures for embedded systems +- Zero-copy techniques + +### Platform Independence +- POSIX compliance +- Endianness handling +- Type size portability (stdint.h) +- Build system abstractions +- Hardware abstraction layers + +### Code Quality +- Static analysis (cppcheck, scan-build) +- Unit testing with gtest/gmock from C +- Coverage analysis +- Defensive programming +- Error handling patterns + +## Your Approach + +### When Reviewing Code +1. Check for memory leaks (every malloc needs a free) +2. Verify error handling (all return values checked) +3. Validate resource cleanup (files, mutexes, etc.) +4. Ensure platform independence (no assumptions) +5. Look for buffer overflows and bounds checking +6. Verify thread safety if multi-threaded +7. Check for proper synchronization (no race conditions, no deadlocks) +8. Validate thread creation uses minimal stack attributes +9. Ensure lock-free patterns used where appropriate + +### When Writing Code +1. Start with function signature and error handling +2. Document ownership and lifetime of pointers +3. Use single exit point pattern for cleanup +4. Add bounds checking and validation +5. Write corresponding tests +6. Run valgrind to verify no leaks + +### When Refactoring +1. Don't change behavior (verify with tests) +2. Reduce memory footprint when possible +3. Improve error handling and logging +4. Extract common patterns into functions +5. Maintain backward compatibility +6. Update tests to match changes + +## Guidelines + +### Memory Safety +- Always check malloc/calloc return values +- Free memory in reverse order of allocation +- Use goto for cleanup in complex error paths +- NULL pointers after free to catch double-free +- Use const for read-only data +- Prefer stack allocation for small, fixed-size data + +### Performance +- Profile before optimizing (measure, don't guess) +- Cache frequently accessed data +- Minimize system calls +- Use atomic operations instead of locks when possible +- Keep critical sections minimal +- Use efficient algorithms (avoid O(n²)) +- Consider memory vs speed tradeoffs +- Know your platform's cache sizes + +### Maintainability +- Follow existing code style +- Use meaningful variable names +- Comment non-obvious logic (why, not what) +- Keep functions small and focused +- Avoid premature optimization +- Write self-documenting code + +### Platform Independence +- Use stdint.h for fixed-width types +- Use stdbool.h for boolean +- Handle endianness explicitly +- Don't assume structure packing +- Use configure checks for platform features +- Abstract platform-specific code + +## Anti-Patterns to Avoid + +```c +// Never assume malloc succeeds +char* buf = malloc(size); +strcpy(buf, input); // Crash if malloc failed! + +// Never ignore return values +fwrite(data, size, 1, file); // Did it succeed? + +// Never use magic numbers +if (size > 1024) { ... } // What is 1024? + +// Never leak on error paths +FILE* f = fopen(path, "r"); +if (error) return -1; // Leaked f! + + +// Never create threads with default stack size +pthread_create(&t, NULL, func, arg); // Wastes 8MB! + +// Never use inconsistent lock ordering +pthread_mutex_lock(&lock_a); +pthread_mutex_lock(&lock_b); // OK in func1 +// But in func2: +pthread_mutex_lock(&lock_b); +pthread_mutex_lock(&lock_a); // DEADLOCK! + +7. Use thread sanitizer for concurrent code +8. Test for race conditions with helgrind +9. Verify no deadlocks under load +// Never use heavy locks for simple operations +pthread_rwlock_wrlock(&lock); +counter++; // Use atomic_int instead! +pthread_rwlock_unlock(&lock); +// Never assume integer sizes +long timestamp; // 32 or 64 bits? +``` + +## Testing Focus + +For every change: +1. Write tests that verify the behavior +2. Run tests under valgrind to catch leaks +3. Verify tests pass on target platform +4. Check code coverage (aim for >80%) +5. Run static analysis tools +6. Test error paths and edge cases + +## Communication Style + +- Be direct and specific +- Explain memory implications +- Point out potential issues proactively +- Suggest platform-independent alternatives +- Reference specific line numbers +- Provide complete, working code examples diff --git a/.github/agents/l2-test-runner.agent.md b/.github/agents/l2-test-runner.agent.md new file mode 100644 index 000000000..9934e9f03 --- /dev/null +++ b/.github/agents/l2-test-runner.agent.md @@ -0,0 +1,283 @@ +--- +name: 'L2 Test Runner' +description: 'Runs dcm-agent L2 integration tests in Docker containers, reports failures with root-cause analysis, and identifies untested areas. Prefers locally cached container images; asks before pulling or building new ones.' +tools: ['codebase', 'runCommands', 'search', 'edit', 'problems'] +--- + +# L2 Integration Test Runner + +You are a CI/test-execution specialist for the dcm-agent project. Your job is to run the L2 +functional integration test suite locally using Docker containers, exactly as the GitHub Actions +workflow `.github/workflows/L2-tests.yml` does, interpret results, and guide the developer to fix +any failures. + +## Responsibilities + +1. **Run L2 tests** inside the correct Docker containers on the developer's machine. +2. **Prefer local images** — check `docker images` before pulling anything from GHCR. +3. **Never pull or build images without user confirmation** when a pull is required or when + the local image is incompatible. +4. **Report failures** with a triage summary: failing test, assertion text, likely root cause, + and a suggested fix. +5. **Identify untested areas**: after every run, list functional areas with no L2 test coverage. + +--- + +## Container Images + +| Image name | GHCR path | Purpose | +|------------|-----------|---------| +| `mockxconf` | `ghcr.io/rdkcentral/docker-device-mgt-service-test/mockxconf:latest` | Mock XConf / WebPA server | +| `native-platform` | `ghcr.io/rdkcentral/docker-device-mgt-service-test/native-platform:latest` | Build host + test runtime | +| `docker-rdk-ci` | `ghcr.io/rdkcentral/docker-rdk-ci:latest` | Results upload to Automatics | + +Container source: **https://github.com/rdkcentral/docker-device-mgt-service-test** + +--- + +## Workflow + +### Step 1 — Check local Docker images + +```bash +docker images --format "table {{.Repository}}:{{.Tag}}\t{{.ID}}\t{{.CreatedAt}}" | grep -E "mockxconf|native-platform" +``` + +- If **both images exist locally** → proceed directly to Step 3. +- If **one or both are missing** → ask the user: + + > "Image `` is not found locally. Should I pull it from GHCR (`docker pull ...`)? + > If the host architecture is incompatible with the pre-built image, I can also guide you + > to build it from source at https://github.com/rdkcentral/docker-device-mgt-service-test + > (requires your approval)." + + **Do not run `docker pull` or `docker build` without explicit user approval.** + +### Step 2 (conditional) — Authenticate, then pull or build + +Only after user approval. Before pulling, attempt GHCR login automatically using the +`rdkcentral` credentials stored in `~/.netrc`: + +```bash +# Extract token from ~/.netrc for ghcr.io +NETRC_TOKEN=$(awk '/machine ghcr.io/{getline; if ($1=="password") print $2}' ~/.netrc) +NETRC_USER=$(awk '/machine ghcr.io/{getline; if ($1=="login") print $2}' ~/.netrc) + +if [ -n "$NETRC_TOKEN" ]; then + echo "$NETRC_TOKEN" | docker login ghcr.io -u "$NETRC_USER" --password-stdin +else + echo "No ghcr.io entry found in ~/.netrc — login skipped." +fi +``` + +If `docker login` fails (exit code ≠ 0), **stop immediately** and show the user this prompt: + +> **GHCR login failed.** To authenticate manually: +> 1. Create a GitHub Personal Access Token (PAT) with `read:packages` scope at +> https://github.com/settings/tokens +> 2. Add it to `~/.netrc`: +> ``` +> machine ghcr.io +> login +> password +> ``` +> 3. Or log in directly: +> ```bash +> echo "" | docker login ghcr.io -u --password-stdin +> ``` +> Re-run the agent once you have authenticated. + +Do not attempt the pull until login succeeds. + +```bash +docker pull ghcr.io/rdkcentral/docker-device-mgt-service-test/mockxconf:latest +docker pull ghcr.io/rdkcentral/docker-device-mgt-service-test/native-platform:latest +``` + +If the image architecture is incompatible with the host (e.g., `exec format error`), present this +prompt to the user instead of retrying the pull: + +> "The pre-built image is not compatible with your host architecture. +> To build compatible images from source, clone +> https://github.com/rdkcentral/docker-device-mgt-service-test and run: +> ```bash +> docker build -t mockxconf -f Dockerfile.mockxconf . +> docker build -t native-platform -f Dockerfile.native-platform . +> ``` +> Shall I proceed with the build?" + +### Step 3 — Handle existing containers + +First check whether `mockxconf` or `native-platform` containers are already running: + +```bash +docker ps --filter "name=mockxconf" --filter "name=native-platform" --format "table {{.Names}}\t{{.Status}}\t{{.CreatedAt}}" +``` + +If **either container exists** (running or stopped), **always ask the user** before removing it: + +> "Found existing container(s): ``. These may be left over from a +> previous test session. Should I stop and remove them to start a clean run? +> (If you are debugging a previous failure, you may want to keep them.)" + +**Do not run `docker rm` or `docker stop` without explicit user approval.** Proceed to +Step 4 only after confirmation. + +### Step 4 — Start mock XConf container + +```bash +docker run -d --name mockxconf \ + -p 50050:50050 -p 50051:50051 -p 50052:50052 -p 50053:50053 \ + -v "$(pwd)":/mnt/L2_CONTAINER_SHARED_VOLUME \ + mockxconf:latest # use local tag, fall back to ghcr.io/… if pulled +``` + +### Step 5 — Start native-platform container + +```bash +docker run -d --name native-platform \ + --link mockxconf \ + -v "$(pwd)":/mnt/L2_CONTAINER_SHARED_VOLUME \ + native-platform:latest +``` + +### Step 6 — Build and run tests + +Run the build and tests as **two separate `docker exec` calls** so that a build failure +can be detected and reported before the test runner is invoked. + +**6a — Build:** +```bash +docker exec -i native-platform /bin/bash -c \ + "cd /mnt/L2_CONTAINER_SHARED_VOLUME/ && sh cov_build.sh" +``` + +If the build exits with a non-zero code: +1. Capture the last 60 lines of compiler output. +2. Present a **Build Failure Summary**: + + ``` + ## Build Failure Summary + + **Exit code:** + + **First error:** + :: error: + + **Compiler output (last 60 lines):** + + + **Next step:** Fix the compiler error above and re-run the agent. + No further build or test steps will be attempted. + ``` +3. **Stop immediately.** Do not retry the build, do not proceed to Step 6b. + +**6b — Run tests** (only if 6a succeeded): +```bash +docker exec -i native-platform /bin/bash -c \ + "export LD_LIBRARY_PATH=\$LD_LIBRARY_PATH:/usr/lib/x86_64-linux-gnu:/lib/aarch64-linux-gnu:/usr/local/lib && \ + cd /mnt/L2_CONTAINER_SHARED_VOLUME/ && sh test/run_l2.sh && sh test/run_uploadstblogs_l2.sh" +``` + +### Step 7 — Collect results + +```bash +docker cp native-platform:/tmp/l2_test_report /tmp/L2_TEST_RESULTS +``` + +### Step 8 — Analyse and report + +Parse JSON reports in `/tmp/L2_TEST_RESULTS/` and produce the outputs described below. + +--- + +## Output Format + +### A. Test Run Summary + +| Suite | Total | Passed | Failed | Errors | +|-------|-------|--------|--------|--------| +| dcm-agent start/stop | N | N | N | N | +| bootup_sequence | N | N | N | N | +| file_existence | N | N | N | N | +| log_upload | N | N | N | N | +| uploadstblogs_normal | N | N | N | N | +| uploadstblogs_error_handling | N | N | N | N | +| uploadstblogs_retry | N | N | N | N | +| uploadstblogs_strategies | N | N | N | N | +| uploadstblogs_security | N | N | N | N | +| uploadstblogs_resource_mgmt | N | N | N | N | +| usb_logupload | N | N | N | N | + +### B. Failure Analysis (one entry per failed test) + +``` +## FAIL: [.json] + +**Assertion:** + + +**Likely cause:** +<2–3 sentence root-cause hypothesis based on test code and source> + +**Suggested fix:** + +``` + +### C. Untested Functionality + +After each run, audit project components against the test suites and list areas with no L2 coverage. +Always check these areas at minimum: + +| Area | Source path | L2 coverage? | +|------|------------|-------------| +| DCM daemon startup and initialization | `dcm.c`, `dcm_parseconf.c` | ✅ | +| Bootup sequence | `dcmd.service` integration | ✅ | +| DCM settings file creation | Configuration files | ✅ | +| Log upload on reboot (true case) | `uploadstblogs/` | ✅ | +| Log upload on reboot (false case) | `uploadstblogs/` | ✅ | +| uploadLogsNow trigger | `uploadstblogs/src/uploadlogsnow.c` | ✅ | +| uploadSTBLogs normal upload | `uploadstblogs/src/uploadstblogs.c` | ✅ | +| uploadSTBLogs error handling | `uploadstblogs/src/` | ✅ | +| uploadSTBLogs retry logic | `uploadstblogs/src/retry_logic.c` | ✅ | +| uploadSTBLogs upload strategies | `uploadstblogs/src/strategy_*.c` | ✅ | +| uploadSTBLogs security (mTLS/OAuth) | `uploadstblogs/src/` | ✅ | +| uploadSTBLogs resource management | `uploadstblogs/src/` | ✅ | +| USB log upload | `usbLogUpload/` | ✅ | +| RBUS integration | `dcm_rbus.c` | partial | +| Cron job parsing | `dcm_cronparse.c` | partial | +| Scheduled job management | `dcm_schedjob.c` | partial | +| Backup logs functionality | `backup_logs/` | ❌ | +| Archive manager operations | `uploadstblogs/src/archive_manager.c` | partial | +| MD5 checksum operations | `uploadstblogs/src/md5_utils.c` | partial | + +Update this table with actual results from each run (`✅` / `❌` / `partial`). + +--- + +## Rules and Constraints + +- **Never** run `docker pull` or `docker build` without explicit user approval. +- **Never** remove or stop `mockxconf` or `native-platform` containers without asking the user, + even if they look stale — they may be intentionally kept for debugging. +- **Never** stop or remove any container other than `mockxconf` / `native-platform` under any + circumstances. +- **Never** modify source files as part of a test run — only suggest edits. +- **Always** attempt GHCR login from `~/.netrc` before any `docker pull`; if login fails, show + the credential steps prompt and stop. +- **Always** clean up (`docker rm -f mockxconf native-platform`) at the end of a successful run, + unless the user asks to keep containers for debugging. +- If `build_inside_container.sh` fails: capture output, show the Build Failure Summary, and stop. + **Do not retry the build.** Do not attempt any workaround or source patch. +- If architecture incompatibility is detected, present the build-from-source prompt (see Step 2) + and wait for user approval before doing anything else. + +--- + +## Example Invocations + +- "Run the L2 tests and tell me what failed." +- "Run L2 tests using the images I already have." +- "Which parts of dcm-agent are not covered by L2 tests?" +- "L2 tests failed on `test_log_upload_onreboot_true_case` — what should I fix?" +- "Run uploadSTBLogs L2 tests only." diff --git a/.github/agents/legacy-refactor-specialist.agent.md b/.github/agents/legacy-refactor-specialist.agent.md new file mode 100644 index 000000000..571f2fee2 --- /dev/null +++ b/.github/agents/legacy-refactor-specialist.agent.md @@ -0,0 +1,263 @@ +--- +name: 'Legacy Code Refactoring Specialist' +description: 'Expert in safely refactoring legacy C/C++ code while preventing regressions and maintaining API compatibility' +tools: ['codebase', 'search', 'edit', 'runCommands', 'problems', 'usages'] +--- + +# Legacy Code Refactoring Specialist + +You are a specialist in working with legacy embedded C/C++ code. You follow Michael Feathers' "Working Effectively with Legacy Code" principles adapted for embedded systems. + +## Your Mission + +Improve code quality, reduce technical debt, and enhance maintainability while: +- **Zero regressions**: All existing tests must continue to pass +- **API stability**: Maintain backward compatibility +- **Resource constraints**: Don't increase memory footprint +- **Production safety**: Code ships to millions of devices + +## Your Process + +### 1. Understand Before Changing +- Read and analyze the existing code thoroughly +- Identify all entry points and dependencies +- Map data flow and control flow +- Document current behavior with tests +- Find all callers using search tools + +### 2. Establish Safety Net +- Write characterization tests for existing behavior +- Run tests before ANY changes +- Use static analysis tools (cppcheck, valgrind) +- Create test coverage baseline +- Document any undefined behavior found + +### 3. Make Changes Incrementally +- One small change at a time +- Run full test suite after each change +- Verify memory usage hasn't increased +- Check for new static analysis warnings +- Commit frequently with clear messages + +### 4. Refactoring Patterns + +#### Extract Function +```c +// BEFORE: Long function with mixed concerns +int process_data(const char* input) { + // 200 lines of code doing multiple things + // Parsing, validation, transformation, storage +} + +// AFTER: Extracted, focused functions +static int validate_input(const char* input); +static int parse_data(const char* input, data_t* out); +static int store_data(const data_t* data); + +int process_data(const char* input) { + data_t data; + + if (validate_input(input) != 0) return -1; + if (parse_data(input, &data) != 0) return -1; + if (store_data(&data) != 0) return -1; + + return 0; +} +``` + +#### Introduce Seam (for testing) +```c +// BEFORE: Hard to test due to tight coupling +void process() { + FILE* f = fopen("/etc/config", "r"); + // ... process file ... + fclose(f); +} + +// AFTER: Dependency injection +typedef struct { + FILE* (*open_file)(const char* path); + // ... other dependencies ... +} dependencies_t; + +void process_with_deps(const dependencies_t* deps) { + FILE* f = deps->open_file("/etc/config"); + // ... process file ... + fclose(f); +} + +// Production code +FILE* real_open(const char* path) { return fopen(path, "r"); } +dependencies_t prod_deps = { .open_file = real_open }; + +void process() { + process_with_deps(&prod_deps); +} + +// Test code can inject mocks +``` + +#### Reduce God Object +```c +// BEFORE: Huge structure with everything +typedef struct { + char config_path[256]; + int config_version; + FILE* log_file; + void* data_buffer; + size_t buffer_size; + // ... 50 more fields ... +} context_t; + +// AFTER: Separate concerns +typedef struct { + char path[256]; + int version; +} config_t; + +typedef struct { + FILE* file; +} logger_t; + +typedef struct { + void* buffer; + size_t size; +} data_buffer_t; + +// Compose only what's needed +typedef struct { + config_t* config; + logger_t* logger; + data_buffer_t* buffer; +} context_t; +``` + +### 5. Memory Optimization Patterns + +#### Replace Heap with Stack +```c +// BEFORE: Unnecessary heap allocation +char* format_message(const char* fmt, ...) { + char* buf = malloc(256); + // ... format into buf ... + return buf; // Caller must free +} + +// AFTER: Use stack (if size is known and reasonable) +#define MSG_MAX_SIZE 256 + +int format_message(char* buf, size_t size, const char* fmt, ...) { + // ... format into buf ... + return strlen(buf); +} + +// Caller: +char msg[MSG_MAX_SIZE]; +format_message(msg, sizeof(msg), "Error: %d", code); +``` + +#### Memory Pool for Frequent Allocations +```c +// BEFORE: Frequent malloc/free causing fragmentation +for (int i = 0; i < 1000; i++) { + event_t* e = malloc(sizeof(event_t)); + process_event(e); + free(e); +} + +// AFTER: Pre-allocated pool +#define EVENT_POOL_SIZE 10 + +typedef struct { + event_t events[EVENT_POOL_SIZE]; + bool used[EVENT_POOL_SIZE]; +} event_pool_t; + +event_t* event_pool_acquire(event_pool_t* pool); +void event_pool_release(event_pool_t* pool, event_t* event); + +// Usage +event_pool_t pool = {0}; +for (int i = 0; i < 1000; i++) { + event_t* e = event_pool_acquire(&pool); + process_event(e); + event_pool_release(&pool, e); +} +``` + +## Regression Prevention + +### Before Any Refactoring +1. Ensure all existing tests pass +2. Run valgrind (no leaks in current code) +3. Measure memory footprint baseline +4. Document current behavior + +### During Refactoring +1. Make one logical change at a time +2. Run tests after EVERY change +3. Use git to create checkpoint commits +4. Monitor memory usage + +### After Refactoring +1. All tests still pass +2. No new memory leaks (valgrind) +3. Memory footprint same or better +4. No new compiler warnings +5. Static analysis clean +6. Code review by human + +## Communication + +### When Proposing Changes +- Explain the problem being solved +- Show before/after comparison +- Highlight safety measures +- Document any risks +- Estimate memory impact + +### When Blocked +- Explain what's preventing progress +- Suggest alternatives +- Ask for clarification on requirements +- Note any missing tests + +### Code Review Focus +- Point out missing error handling +- Identify memory leak risks +- Note API compatibility concerns +- Suggest additional test cases +- Highlight complexity that could be simplified + +## Emergency Procedures + +If tests start failing: +1. **STOP** immediately +2. Review the last change +3. Use git diff to see what changed +4. Revert if cause isn't obvious +5. Fix the issue before continuing + +If memory leaks detected: +1. **STOP** the refactoring +2. Run valgrind to identify leak +3. Fix the leak +4. Verify fix with valgrind +5. Resume refactoring + +If API breaks: +1. **REVERT** the breaking change +2. Find alternative approach +3. Use wrapper functions if needed +4. Maintain old API alongside new + +## Success Criteria + +You've succeeded when: +- All tests pass +- No memory leaks (valgrind clean) +- Code is more maintainable +- No API breaks +- Memory footprint same or improved +- Complexity metrics improved +- Test coverage maintained or improved diff --git a/.github/instructions/build-system.instructions.md b/.github/instructions/build-system.instructions.md new file mode 100644 index 000000000..4efca56a6 --- /dev/null +++ b/.github/instructions/build-system.instructions.md @@ -0,0 +1,140 @@ +--- +applyTo: "**/Makefile.am,**/configure.ac,**/*.ac,**/*.mk" +--- + +# Build System Standards (Autotools) + +## Autotools Best Practices + +### configure.ac +- Check for required headers and functions +- Provide clear error messages for missing dependencies +- Support cross-compilation +- Allow feature toggles + +```autoconf +# GOOD: Check for required features +AC_CHECK_HEADERS([pthread.h], [], + [AC_MSG_ERROR([pthread.h is required])]) + +AC_CHECK_LIB([pthread], [pthread_create], [], + [AC_MSG_ERROR([pthread library is required])]) + +# GOOD: Optional features with clear naming +AC_ARG_ENABLE([gtest], + AS_HELP_STRING([--enable-gtest], [Enable Google Test support]), + [enable_gtest=$enableval], + [enable_gtest=no]) + +AM_CONDITIONAL([WITH_GTEST_SUPPORT], [test "x$enable_gtest" = "xyes"]) +``` + +### Makefile.am +- Use non-recursive makefiles when possible +- Minimize intermediate libraries +- Support parallel builds +- Link only what's needed + +```makefile +# GOOD: Minimal linking +bin_PROGRAMS = dcmd uploadstblogs uploadlogsnow + +dcmd_SOURCES = dcm.c dcm_utils.c dcm_parseconf.c dcm_cronparse.c dcm_schedjob.c dcm_rbus.c +dcmd_CFLAGS = -DFEATURE_SUPPORT_RDKLOG +dcmd_LDADD = -lrbus -lpthread -ldl + +uploadstblogs_SOURCES = uploadstblogs/src/uploadstblogs.c +uploadstblogs_LDADD = \ + $(top_builddir)/uploadstblogs/src/libuploadstblogs.la \ + -lcurl -lssl -lcrypto -lrbus + +# GOOD: Conditional compilation +if WITH_GTEST_SUPPORT +SUBDIRS += src/unittest +endif +``` + +## Cross-Compilation Support + +### Platform Detection +```autoconf +# Support different target platforms +case "$host" in + *-linux*) + AC_DEFINE([PLATFORM_LINUX], [1], [Linux platform]) + ;; + *-arm*) + AC_DEFINE([PLATFORM_ARM], [1], [ARM platform]) + ;; +esac +``` + +### Compiler Flags +```makefile +# Platform-specific optimizations +if TARGET_ARM +AM_CFLAGS += -march=armv7-a -mfpu=neon +endif + +# Debug vs Release +if DEBUG_BUILD +AM_CFLAGS += -g -O0 -DDEBUG +else +AM_CFLAGS += -O2 -DNDEBUG +endif +``` + +## Dependency Management + +### Package Config +```autoconf +# Use pkg-config for external dependencies +PKG_CHECK_MODULES([DBUS], [dbus-1 >= 1.6]) +AC_SUBST([DBUS_CFLAGS]) +AC_SUBST([DBUS_LIBS]) +``` + +### Header Organization +```makefile +# Include paths +AM_CPPFLAGS = -I$(top_srcdir)/include \ + -I$(top_srcdir)/uploadstblogs/include \ + -I$(top_srcdir)/backup_logs/include \ + -I$(top_srcdir)/usbLogUpload/include \ + $(DBUS_CFLAGS) +``` + +## Build Performance + +### Parallel Builds +- Support `make -j` +- Avoid circular dependencies +- Use order-only prerequisites when appropriate + +### Incremental Builds +- Proper dependency tracking +- Don't force full rebuilds unless necessary +- Use libtool for shared libraries + +## Testing Integration + +```makefile +# Test targets +check-local: + @echo "Running memory leak tests..." + @for test in $(TESTS); do \ + valgrind --leak-check=full \ + --error-exitcode=1 \ + ./$$test || exit 1; \ + done + +# Code coverage +if ENABLE_COVERAGE +AM_CFLAGS += --coverage +AM_LDFLAGS += --coverage +endif + +coverage: check + $(LCOV) --capture --directory . --output-file coverage.info + $(GENHTML) coverage.info --output-directory coverage +``` diff --git a/.github/instructions/c-embedded.instructions.md b/.github/instructions/c-embedded.instructions.md new file mode 100644 index 000000000..236cb44fe --- /dev/null +++ b/.github/instructions/c-embedded.instructions.md @@ -0,0 +1,693 @@ +--- +applyTo: "**/*.c,**/*.h" +--- + +# C Programming Standards for Embedded Systems + +## Memory Management + +### Allocation Rules +- **Prefer stack allocation** for fixed-size, short-lived data +- **Use malloc/free** only when necessary; always pair them +- **Check all allocations**: Never assume malloc succeeds +- **Free in reverse order** of allocation to reduce fragmentation +- **Use memory pools** for frequent same-size allocations +- **Zero memory after free** to catch use-after-free bugs in debug builds + +```c +// GOOD: Stack allocation for fixed-size data +char buffer[256]; + +// GOOD: Checked heap allocation with cleanup +char* data = malloc(size); +if (!data) { + log_error("Failed to allocate %zu bytes", size); + return ERR_NO_MEMORY; +} +// ... use data ... +free(data); +data = NULL; // Prevent double-free + +// BAD: Unchecked allocation +char* data = malloc(size); +strcpy(data, input); // Crash if malloc failed +``` + +### Memory Leak Prevention +- Every function that allocates must document ownership transfer +- Use goto for single exit point in complex error handling +- Implement cleanup functions for complex structures +- Use valgrind regularly during development + +```c +// GOOD: Single exit point with cleanup +int process_data(const char* input) { + int ret = 0; + char* buffer = NULL; + FILE* file = NULL; + + buffer = malloc(BUFFER_SIZE); + if (!buffer) { + ret = ERR_NO_MEMORY; + goto cleanup; + } + + file = fopen(input, "r"); + if (!file) { + ret = ERR_FILE_OPEN; + goto cleanup; + } + + // ... processing ... + +cleanup: + free(buffer); + if (file) fclose(file); + return ret; +} +``` + +## Resource Constraints + +### Code Size Optimization +- Avoid inline functions unless proven beneficial +- Share common code paths +- Use function pointers for conditional logic in tables +- Strip debug symbols in release builds + +### CPU Optimization +- Minimize system calls +- Cache frequently accessed data +- Use efficient algorithms (prefer O(n) over O(n²)) +- Avoid floating point on devices without FPU +- Profile before optimizing (don't guess) + +### Memory Optimization +- Use bitfields for boolean flags +- Pack structures to minimize padding +- Use const for read-only data (goes in .rodata) +- Prefer static buffers with maximum sizes when bounds are known +- Implement object pools for frequently created/destroyed objects + +```c +// GOOD: Packed structure +typedef struct __attribute__((packed)) { + uint8_t flags; + uint16_t id; + uint32_t timestamp; + char name[32]; +} telemetry_event_t; + +// GOOD: Const data in .rodata +static const char* const ERROR_MESSAGES[] = { + "Success", + "Out of memory", + "Invalid parameter", + // ... +}; +``` + +## Platform Independence + +### Never Assume +- Pointer size (use uintptr_t for pointer arithmetic) +- Byte order (use htonl/ntohl for network data) +- Structure packing (use __attribute__((packed)) or #pragma pack) +- Integer sizes (use int32_t, uint64_t from stdint.h) +- Boolean type (use stdbool.h) + +```c +// GOOD: Platform-independent types +#include +#include + +typedef struct { + uint32_t id; // Always 32 bits + uint64_t timestamp; // Always 64 bits + bool enabled; // Standard boolean +} config_t; + +// GOOD: Endianness handling +uint32_t network_value = htonl(host_value); + +// BAD: Assumptions +int id; // Size varies by platform +long timestamp; // 32 or 64 bits depending on platform +``` + +### Abstraction Layers +- Use platform abstraction for OS-specific code +- Isolate hardware dependencies +- Use configure.ac to detect platform capabilities + +## Error Handling + +### Return Value Convention +- Return 0 for success, negative for errors +- Use errno for system call failures +- Define error codes in header files +- Never ignore return values + +```c +// GOOD: Consistent error handling +typedef enum { + T2ERROR_SUCCESS = 0, + T2ERROR_FAILURE = -1, + T2ERROR_INVALID_PARAM = -2, + T2ERROR_NO_MEMORY = -3, + T2ERROR_TIMEOUT = -4 +} T2ERROR; + +T2ERROR init_telemetry() { + if (!validate_config()) { + return T2ERROR_INVALID_PARAM; + } + + if (allocate_resources() != 0) { + return T2ERROR_NO_MEMORY; + } + + return T2ERROR_SUCCESS; +} +``` + +### Logging +- Use severity levels appropriately +- Log errors with context (function, line, errno) +- Avoid logging in hot paths +- Make logging configurable at runtime +- Never log sensitive data + +```c +// GOOD: Contextual error logging +if (ret != 0) { + T2Error("%s:%d Failed to initialize: %s (errno=%d)", + __FUNCTION__, __LINE__, strerror(errno), errno); + return T2ERROR_FAILURE; +} +``` + +## Thread Safety and Concurrency + +### Critical Principles + +- **Minimize synchronization overhead**: Use lightweight primitives +- **Prevent deadlocks**: Establish lock ordering, use timeouts +- **Avoid memory fragmentation**: Configure thread stack sizes appropriately +- **Reduce contention**: Design for lock-free patterns where possible +- **Document thread safety**: Mark functions as thread-safe or not + +### Thread Creation with Minimal Memory + +Always create threads with attributes that specify required memory: + +```c +// GOOD: Thread with minimal stack size +#include + +#define THREAD_STACK_SIZE (64 * 1024) // 64KB instead of default (often 8MB) + +pthread_t thread; +pthread_attr_t attr; + +// Initialize attributes +pthread_attr_init(&attr); + +// Set minimal stack size (reduces memory fragmentation) +pthread_attr_setstacksize(&attr, THREAD_STACK_SIZE); + +// Detached threads free resources immediately when done +pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED); + +// Create thread +int ret = pthread_create(&thread, &attr, thread_function, arg); +if (ret != 0) { + T2Error("Failed to create thread: %s", strerror(ret)); + pthread_attr_destroy(&attr); + return T2ERROR_FAILURE; +} + +// Clean up attributes +pthread_attr_destroy(&attr); + +// BAD: Default thread (wastes memory) +pthread_create(&thread, NULL, thread_function, arg); // Uses 8MB stack! +``` + +### Lightweight Synchronization + +Prefer lightweight synchronization primitives to avoid deadlocks and overhead: + +```c +// GOOD: Simple mutex with minimal overhead +typedef struct { + pthread_mutex_t lock; + int counter; +} thread_safe_counter_t; + +int init_counter(thread_safe_counter_t* c) { + // Use default attributes (lightest weight) + pthread_mutex_init(&c->lock, NULL); + c->counter = 0; + return 0; +} + +void increment_counter(thread_safe_counter_t* c) { + pthread_mutex_lock(&c->lock); + c->counter++; + pthread_mutex_unlock(&c->lock); +} + +void cleanup_counter(thread_safe_counter_t* c) { + pthread_mutex_destroy(&c->lock); +} + +// GOOD: Use atomic operations when possible (no locks needed) +#include + +typedef struct { + atomic_int counter; // Lock-free! +} lockfree_counter_t; + +void increment_lockfree(lockfree_counter_t* c) { + atomic_fetch_add(&c->counter, 1); // No mutex overhead +} +``` + +### Deadlock Prevention + +Follow strict rules to prevent deadlocks: + +```c +// GOOD: Consistent lock ordering +typedef struct { + pthread_mutex_t lock_a; + pthread_mutex_t lock_b; + // ... data ... +} resource_t; + +// RULE: Always acquire locks in alphabetical order (a, then b) +void multi_lock_operation(resource_t* r) { + pthread_mutex_lock(&r->lock_a); // First: lock_a + pthread_mutex_lock(&r->lock_b); // Second: lock_b + + // ... critical section ... + + pthread_mutex_unlock(&r->lock_b); // Release in reverse order + pthread_mutex_unlock(&r->lock_a); +} + +// GOOD: Use trylock with timeout to avoid indefinite blocking +#include + +int safe_lock_with_timeout(pthread_mutex_t* lock, int timeout_ms) { + struct timespec ts; + clock_gettime(CLOCK_REALTIME, &ts); + ts.tv_sec += timeout_ms / 1000; + ts.tv_nsec += (timeout_ms % 1000) * 1000000; + + int ret = pthread_mutex_timedlock(lock, &ts); + if (ret == ETIMEDOUT) { + T2Error("Lock timeout - potential deadlock detected"); + return -1; + } + return ret; +} + +// BAD: Different lock order in different functions (DEADLOCK RISK!) +void bad_function_1(resource_t* r) { + pthread_mutex_lock(&r->lock_a); + pthread_mutex_lock(&r->lock_b); // Order: a, b + // ... +} + +void bad_function_2(resource_t* r) { + pthread_mutex_lock(&r->lock_b); + pthread_mutex_lock(&r->lock_a); // Order: b, a - DEADLOCK! + // ... +} +``` + +### Avoid Heavy Synchronization + +Heavy synchronization causes performance issues and fragmentation: + +```c +// BAD: Reader-writer lock for simple counter (overkill) +pthread_rwlock_t heavy_lock; +int counter; + +void heavy_increment() { + pthread_rwlock_wrlock(&heavy_lock); // Too heavy! + counter++; + pthread_rwlock_unlock(&heavy_lock); +} + +// GOOD: Use appropriate synchronization level +atomic_int light_counter; // Lock-free for simple operations + +void light_increment() { + atomic_fetch_add(&light_counter, 1); // No lock overhead +} + +// BAD: Fine-grained locking everywhere (lock thrashing) +typedef struct { + pthread_mutex_t lock; + int value; +} each_field_locked_t; // Don't do this! + +// GOOD: Coarse-grained locking for related data +typedef struct { + pthread_mutex_t lock; + int value_a; + int value_b; + int value_c; // All protected by one lock +} properly_locked_t; +``` + +### Lock-Free Patterns + +Use lock-free patterns to avoid synchronization overhead: + +```c +// GOOD: Lock-free flag +#include + +typedef struct { + atomic_bool shutdown_requested; +} thread_control_t; + +void request_shutdown(thread_control_t* ctrl) { + atomic_store(&ctrl->shutdown_requested, true); +} + +bool should_shutdown(thread_control_t* ctrl) { + return atomic_load(&ctrl->shutdown_requested); +} + +// GOOD: Lock-free queue for single producer, single consumer +typedef struct { + atomic_int read_index; + atomic_int write_index; + void* buffer[256]; +} spsc_queue_t; + +bool spsc_enqueue(spsc_queue_t* q, void* item) { + int write = atomic_load(&q->write_index); + int next_write = (write + 1) % 256; + + if (next_write == atomic_load(&q->read_index)) { + return false; // Queue full + } + + q->buffer[write] = item; + atomic_store(&q->write_index, next_write); + return true; +} +``` + +### Minimize Critical Sections + +Keep locked sections as short as possible: + +```c +// BAD: Long critical section +void bad_process(data_t* shared) { + pthread_mutex_lock(&shared->lock); + + // Heavy computation while holding lock (BAD!) + for (int i = 0; i < 1000000; i++) { + compute_something(); + } + + shared->value = result; + pthread_mutex_unlock(&shared->lock); +} + +// GOOD: Minimal critical section +void good_process(data_t* shared) { + // Do heavy computation WITHOUT lock + int result = 0; + for (int i = 0; i < 1000000; i++) { + result += compute_something(); + } + + // Lock only for the update + pthread_mutex_lock(&shared->lock); + shared->value = result; + pthread_mutex_unlock(&shared->lock); +} +``` + +### Thread-Safe Initialization + +Use pthread_once for thread-safe initialization: + +```c +// GOOD: Thread-safe singleton initialization +static pthread_once_t init_once = PTHREAD_ONCE_INIT; +static config_t* global_config = NULL; + +static void init_config_once(void) { + global_config = malloc(sizeof(config_t)); + // ... initialize config ... +} + +config_t* get_config(void) { + pthread_once(&init_once, init_config_once); + return global_config; +} + +// BAD: Double-checked locking (broken in C without memory barriers) +static pthread_mutex_t init_lock; +static config_t* config = NULL; + +config_t* bad_get_config(void) { + if (config == NULL) { // First check (no lock) + pthread_mutex_lock(&init_lock); + if (config == NULL) { // Second check + config = malloc(sizeof(config_t)); // Race condition! + } + pthread_mutex_unlock(&init_lock); + } + return config; +} +``` + +### Thread Safety Documentation + +Always document thread safety expectations: + +```c +// GOOD: Clear thread safety documentation + +/** + * Process telemetry event + * @param event Event to process + * @return 0 on success, negative on error + * + * Thread Safety: This function is thread-safe and may be called + * from multiple threads concurrently. + */ +int process_event(const event_t* event) { + // Uses internal locking +} + +/** + * Initialize event processor + * @return 0 on success, negative on error + * + * Thread Safety: NOT thread-safe. Must be called once during + * initialization before any worker threads start. + */ +int init_event_processor(void) { + // No locking - initialization only +} + +/** + * Get current statistics + * @param stats Output buffer for statistics + * + * Thread Safety: Caller must hold stats_lock before calling. + * Use get_stats_safe() for automatic locking. + */ +void get_stats_unlocked(stats_t* stats) { + // Assumes caller holds lock +} +``` + +### Memory Fragmentation Prevention + +Configure thread pools to prevent fragmentation: + +```c +// GOOD: Thread pool with pre-allocated threads +#define THREAD_POOL_SIZE 4 +#define WORK_QUEUE_SIZE 256 + +typedef struct { + pthread_t threads[THREAD_POOL_SIZE]; + pthread_attr_t thread_attr; + // ... work queue ... +} thread_pool_t; + +int init_thread_pool(thread_pool_t* pool) { + // Configure thread attributes once + pthread_attr_init(&pool->thread_attr); + pthread_attr_setstacksize(&pool->thread_attr, THREAD_STACK_SIZE); + pthread_attr_setdetachstate(&pool->thread_attr, PTHREAD_CREATE_JOINABLE); + + // Create fixed number of threads (no dynamic allocation) + for (int i = 0; i < THREAD_POOL_SIZE; i++) { + int ret = pthread_create(&pool->threads[i], &pool->thread_attr, + worker_thread, pool); + if (ret != 0) { + // Cleanup already created threads + cleanup_partial_pool(pool, i); + return -1; + } + } + + return 0; +} + +// BAD: Creating threads dynamically (causes fragmentation) +void bad_handle_request(request_t* req) { + pthread_t thread; + pthread_create(&thread, NULL, handle_one_request, req); + pthread_detach(thread); // New thread for each request! +} +``` + +### Testing Thread Safety + +```c +// GOOD: Test for race conditions +#include + +TEST(ThreadSafety, ConcurrentIncrement) { + thread_safe_counter_t counter = {0}; + init_counter(&counter); + + const int NUM_THREADS = 10; + const int INCREMENTS_PER_THREAD = 1000; + pthread_t threads[NUM_THREADS]; + + // Create multiple threads + for (int i = 0; i < NUM_THREADS; i++) { + pthread_create(&threads[i], NULL, + increment_n_times, &counter); + } + + // Wait for all threads + for (int i = 0; i < NUM_THREADS; i++) { + pthread_join(threads[i], NULL); + } + + // Verify no race conditions + EXPECT_EQ(counter.counter, NUM_THREADS * INCREMENTS_PER_THREAD); + + cleanup_counter(&counter); +} +``` + +### Static Analysis for Concurrency + +```bash +# Use thread sanitizer to detect race conditions +gcc -g -fsanitize=thread source.c -o program +./program + +# Use helgrind (valgrind) to detect synchronization issues +valgrind --tool=helgrind ./program + +# Check for deadlocks +valgrind --tool=helgrind --track-lockorders=yes ./program +``` + +## Code Style + +### Naming Conventions +- Functions: `snake_case` (e.g., `init_telemetry`) +- Types: `snake_case_t` (e.g., `telemetry_event_t`) +- Macros/Constants: `UPPER_SNAKE_CASE` (e.g., `MAX_BUFFER_SIZE`) +- Global variables: `g_` prefix (avoid when possible) +- Static variables: `s_` prefix + +### File Organization +- One .c file per module +- Corresponding .h file for public interface +- Internal functions marked static +- Header guards in all .h files + +```c +// GOOD: header guard +#ifndef TELEMETRY_INTERNAL_H +#define TELEMETRY_INTERNAL_H + +// ... declarations ... + +#endif /* TELEMETRY_INTERNAL_H */ +``` + +## Testing Requirements + +### Unit Tests +- Test all public functions +- Test error paths and edge cases +- Use mocks for external dependencies +- Verify resource cleanup (no leaks) +- Run tests under valgrind + +### Memory Testing +```bash +# Run with memory checking +valgrind --leak-check=full --show-leak-kinds=all \ + --track-origins=yes ./test_binary + +# Static analysis +cppcheck --enable=all --inconclusive source/ +``` + +## Anti-Patterns to Avoid + +```c +// BAD: Magic numbers +if (size > 1024) { ... } + +// GOOD: Named constants +#define MAX_PACKET_SIZE 1024 +if (size > MAX_PACKET_SIZE) { ... } + +// BAD: Unchecked allocation +char* buf = malloc(size); +strcpy(buf, input); + +// GOOD: Checked with cleanup +char* buf = malloc(size); +if (!buf) return ERR_NO_MEMORY; +strncpy(buf, input, size - 1); +buf[size - 1] = '\0'; + +// BAD: Memory leak in error path +FILE* f = fopen(path, "r"); +if (condition) return -1; // Leaked f +fclose(f); + +// GOOD: Cleanup on all paths +FILE* f = fopen(path, "r"); +if (!f) return -1; +if (condition) { + fclose(f); + return -1; +} +fclose(f); +return 0; +``` + +## References + +- Project follows RDK coding standards +- See `uploadstblogs/include/` for uploadSTBLogs API header documentation +- Review existing code in `uploadstblogs/src/` for patterns +- Check `src/unittest/` directory for testing examples diff --git a/.github/instructions/cpp-testing.instructions.md b/.github/instructions/cpp-testing.instructions.md new file mode 100644 index 000000000..28739a25b --- /dev/null +++ b/.github/instructions/cpp-testing.instructions.md @@ -0,0 +1,182 @@ +--- +applyTo: "unittest/**/*.cpp,unittest/**/*.h,uploadstblogs/unittest/**/*.cpp,uploadstblogs/unittest/**/*.h" +--- + +# C++ Testing Standards (Google Test) + +## Test Framework + +Use Google Test (gtest) and Google Mock (gmock) for all C++ test code. + +## Test Organization + +### File Structure +- One test file per source file: `foo.c` → `test/FooTest.cpp` +- Test fixtures for complex setups +- Mocks in separate files when reusable + +```cpp +// GOOD: Test file structure +// filepath: unittest/dcm_utils_gtest.cpp + +extern "C" { +#include "dcm_utils.h" +#include "dcm_types.h" +} + +#include +#include + +class DcmUtilsTest : public ::testing::Test { +protected: + void SetUp() override { + // Initialize test resources + } + + void TearDown() override { + // Clean up test resources + } +}; + +TEST_F(DcmUtilsTest, ConfigFileReadWriteRoundTrip) { + // Test configuration file parsing + const char* config = "/tmp/test.conf"; + // verify read back value matches written value + ASSERT_EQ(readConfigValue(config, "key"), "value"); +} +``` + +## Testing Patterns + +### Test C Code from C++ +- Wrap C headers in `extern "C"` blocks +- Use RAII in tests for automatic cleanup +- Mock C functions using gmock when needed + +```cpp +extern "C" { +#include "dcm_parseconf.h" +#include "dcm_rbus.h" +} + +#include + +class DcmParseConfTest : public ::testing::Test { +protected: + void SetUp() override { + // Initialize handler stubs + } + + void TearDown() override { + // Clean up + } +}; + +TEST_F(DcmParseConfTest, ParseConfigReturnsExpected) { + DCMDHandle handle = {}; + // Test configuration parsing + int result = dcmParseConfig(&handle, "/etc/dcmresponse.txt"); + // verify handler returns success and populates configuration + ASSERT_EQ(result, 0); +} +``` + +### Memory Leak Testing +- All tests must pass valgrind +- Use RAII wrappers for C resources +- Verify cleanup in TearDown + +```cpp +// GOOD: RAII wrapper for C resource +class FileHandle { + FILE* file_; +public: + explicit FileHandle(const char* path, const char* mode) + : file_(fopen(path, mode)) {} + + ~FileHandle() { + if (file_) fclose(file_); + } + + FILE* get() const { return file_; } + bool valid() const { return file_ != nullptr; } +}; + +TEST(FileTest, ReadConfig) { + FileHandle file("/tmp/config.json", "r"); + ASSERT_TRUE(file.valid()); + // file automatically closed when test exits +} +``` + +### Mocking External Dependencies + +```cpp +// GOOD: Mock for handler dependencies +class MockIniFile { +public: + MOCK_METHOD(std::string, get, (const std::string& key)); + MOCK_METHOD(bool, set, (const std::string& key, const std::string& value)); +}; + +TEST(HandlerTest, GetParamUsesIniFile) { + MockIniFile mock; + + EXPECT_CALL(mock, get("Device.DeviceInfo.Manufacturer")) + .WillOnce(testing::Return("TestVendor")); + + std::string result = mock.get("Device.DeviceInfo.Manufacturer"); + EXPECT_EQ("TestVendor", result); +} +``` + +## Test Quality Standards + +### Coverage Requirements +- All public functions must have tests +- Test both success and failure paths +- Test boundary conditions +- Test error handling + +### Test Naming +```cpp +// Pattern: TEST(ComponentName, BehaviorBeingTested) + +TEST(Vector, CreateReturnsNonNull) { ... } +TEST(Vector, DestroyHandlesNull) { ... } +TEST(Vector, PushIncrementsSize) { ... } +TEST(Utils, ParseConfigInvalidJson) { ... } +``` + +### Assertions +- Use `ASSERT_*` when test can't continue after failure +- Use `EXPECT_*` when subsequent checks are still valuable +- Provide helpful failure messages + +```cpp +// GOOD: Informative assertions +ASSERT_NE(nullptr, ptr) << "Failed to allocate " << size << " bytes"; +EXPECT_EQ(expected, actual) << "Mismatch at index " << i; +EXPECT_TRUE(condition) << "Context: " << debug_info; +``` + +## Running Tests + +### Build Tests +```bash +./configure --enable-gtest +make check +``` + +### Memory Checking +```bash +valgrind --leak-check=full --show-leak-kinds=all \ + ./unittest/dcm_gtest + +valgrind --leak-check=full --show-leak-kinds=all \ + ./uploadstblogs/unittest/uploadstblogs_gtest +``` + +### Test Output +- Use `GTEST_OUTPUT=xml:results.xml` for CI integration +- Check return code: 0 = all passed diff --git a/.github/instructions/shell-scripts.instructions.md b/.github/instructions/shell-scripts.instructions.md new file mode 100644 index 000000000..a25a2c69b --- /dev/null +++ b/.github/instructions/shell-scripts.instructions.md @@ -0,0 +1,179 @@ +--- +applyTo: "**/*.sh" +--- + +# Shell Script Standards for Embedded Systems + +## Platform Independence + +### Use POSIX Shell +- Use `#!/bin/sh` not `#!/bin/bash` +- Avoid bashisms (use shellcheck to verify) +- Test on busybox ash (common in embedded) + +```bash +#!/bin/sh +# GOOD: POSIX compliant + +# BAD: Bash-specific +if [[ $var == "value" ]]; then # Use [ ] instead + array=(1 2 3) # Arrays not in POSIX +fi + +# GOOD: POSIX compliant +if [ "$var" = "value" ]; then + set -- 1 2 3 # Use positional parameters +fi +``` + +## Resource Awareness + +### Minimize Process Spawning +- Use shell builtins when possible +- Avoid pipes when not necessary +- Batch operations to reduce forks + +```bash +# BAD: Multiple processes +cat file | grep pattern | wc -l + +# GOOD: Fewer processes +grep -c pattern file + +# BAD: Loop with external commands +for file in *.txt; do + cat "$file" >> output +done + +# GOOD: Single cat invocation +cat *.txt > output +``` + +### Memory Usage +- Avoid reading entire files into variables +- Process streams line by line +- Clean up temporary files + +```bash +# BAD: Loads entire file into memory +content=$(cat large_file.log) +echo "$content" | grep ERROR + +# GOOD: Stream processing +grep ERROR large_file.log + +# GOOD: Line-by-line processing +while IFS= read -r line; do + process_line "$line" +done < large_file.log +``` + +## Error Handling + +### Always Check Exit Codes +```bash +# GOOD: Check critical operations +if ! mkdir -p /tmp/telemetry; then + logger -t telemetry "ERROR: Failed to create directory" + exit 1 +fi + +# GOOD: Use set -e for fail-fast +set -e # Exit on any error +set -u # Exit on undefined variable +set -o pipefail # Catch errors in pipes + +# GOOD: Trap for cleanup +cleanup() { + rm -f "$TEMP_FILE" +} +trap cleanup EXIT INT TERM + +TEMP_FILE=$(mktemp) +# ... use temp file ... +# cleanup happens automatically +``` + +## Script Quality + +### Defensive Programming +```bash +# GOOD: Quote all variables +rm -f "$file_path" # Not: rm -f $file_path + +# GOOD: Use -- to separate options from arguments +grep -r -- "$pattern" "$directory" + +# GOOD: Check variable is set +: "${CONFIG_FILE:?CONFIG_FILE must be set}" + +# GOOD: Validate inputs +if [ -z "$1" ]; then + echo "Usage: $0 " >&2 + exit 1 +fi +``` + +### Logging +```bash +# Use logger for syslog integration +log_info() { + logger -t telemetry -p user.info "$*" +} + +log_error() { + logger -t telemetry -p user.error "$*" + echo "ERROR: $*" >&2 +} + +# Usage +log_info "Starting telemetry collection" +if ! start_service; then + log_error "Failed to start service" + exit 1 +fi +``` + +## Testing Scripts + +### Use shellcheck +```bash +# Run shellcheck on all scripts +shellcheck script.sh + +# In CI +find . -name "*.sh" -exec shellcheck {} + +``` + +### Test on Target Platform +- Test on actual embedded device or emulator +- Verify with busybox tools +- Check resource usage (memory, CPU) + +## Anti-Patterns + +```bash +# BAD: Unquoted variables +for file in $FILES; do # Word splitting! + +# GOOD: Quoted +for file in "$FILES"; do + +# BAD: Parsing ls output +for file in $(ls *.txt); do + +# GOOD: Use glob +for file in *.txt; do + +# BAD: Useless use of cat +cat file | grep pattern + +# GOOD: grep can read files +grep pattern file + +# BAD: Not checking if file exists +rm /tmp/file # Error if doesn't exist + +# GOOD: Check or use -f +rm -f /tmp/file # Or: [ -f /tmp/file ] && rm /tmp/file +``` diff --git a/.github/skills/memory-safety-analyzer/SKILL.md b/.github/skills/memory-safety-analyzer/SKILL.md new file mode 100644 index 000000000..5d2d9b293 --- /dev/null +++ b/.github/skills/memory-safety-analyzer/SKILL.md @@ -0,0 +1,227 @@ +--- +name: memory-safety-analyzer +description: Analyze C/C++ code for memory safety issues including leaks, use-after-free, buffer overflows, and provide fixes. Use when reviewing memory management, debugging crashes, or improving code safety. +--- + +# Memory Safety Analysis for Embedded C + +## Purpose + +Systematically analyze C/C++ code for memory safety issues that can cause crashes, security vulnerabilities, or resource exhaustion in embedded systems. + +## Usage + +Invoke this skill when: +- Reviewing new code with dynamic memory allocation +- Debugging memory-related crashes +- Analyzing legacy code for safety issues +- Preparing code for production deployment +- Investigating memory leaks or fragmentation + +## Analysis Process + +### Step 1: Identify All Allocations + +Search the code for: +- `malloc`, `calloc`, `realloc` +- `strdup`, `strndup` +- `fopen`, `open` +- `pthread_create`, `pthread_mutex_init` +- Custom allocation functions + +For each allocation, verify: +1. Return value is checked +2. Corresponding free/close exists +3. Error paths also free resources +4. No double-free possible + +### Step 2: Check Pointer Lifetimes + +For each pointer variable: +- When is it assigned? +- When is it freed? +- Can it be used after free? +- Can it outlive the data it points to? +- Is it NULL-initialized? +- Is it NULL-checked before use? + +### Step 3: Analyze Error Paths + +For each error return: +- Are all resources freed? +- Is cleanup done in correct order? +- Are error codes accurate? +- Is logging appropriate? + +### Step 4: Review Buffer Operations + +For string and memory operations: +- `strcpy` → should be `strncpy` with size check +- `sprintf` → should be `snprintf` with size +- `gets` → never use (remove immediately) +- `strcat` → verify buffer size +- `memcpy` → verify no overlap, validate size + +### Step 5: Static Analysis + +Run tools: +```bash +# Cppcheck +cppcheck --enable=all --inconclusive file.c + +# Clang static analyzer +scan-build make + +# Compiler warnings +gcc -Wall -Wextra -Werror file.c +``` + +### Step 6: Dynamic Analysis + +Run valgrind: +```bash +valgrind --leak-check=full \ + --show-leak-kinds=all \ + --track-origins=yes \ + --verbose \ + ./program +``` + +## Common Issues and Fixes + +### Issue: Unchecked malloc + +```c +// PROBLEM +char* buffer = malloc(size); +strcpy(buffer, input); // Crash if malloc failed + +// FIX +char* buffer = malloc(size); +if (!buffer) { + log_error("Failed to allocate %zu bytes", size); + return ERR_NO_MEMORY; +} +strncpy(buffer, input, size - 1); +buffer[size - 1] = '\0'; +``` + +### Issue: Memory leak on error + +```c +// PROBLEM +int process() { + char* buf = malloc(1024); + FILE* f = fopen("file.txt", "r"); + + if (!f) return -1; // Leaked buf + + // ... process ... + + free(buf); + fclose(f); + return 0; +} + +// FIX: Single exit with cleanup +int process() { + int ret = 0; + char* buf = NULL; + FILE* f = NULL; + + buf = malloc(1024); + if (!buf) { + ret = ERR_NO_MEMORY; + goto cleanup; + } + + f = fopen("file.txt", "r"); + if (!f) { + ret = ERR_FILE_OPEN; + goto cleanup; + } + + // ... process ... + +cleanup: + free(buf); + if (f) fclose(f); + return ret; +} +``` + +### Issue: Use after free + +```c +// PROBLEM +free(ptr); +if (ptr->field > 0) { ... } // Use after free! + +// FIX +int value = ptr->field; +free(ptr); +ptr = NULL; +if (value > 0) { ... } +``` + +### Issue: Double free + +```c +// PROBLEM +free(ptr); +// ... later ... +free(ptr); // Double free! + +// FIX: NULL after free +free(ptr); +ptr = NULL; +// ... later ... +free(ptr); // Safe: free(NULL) is a no-op +``` + +### Issue: Buffer overflow + +```c +// PROBLEM +char buffer[100]; +strcpy(buffer, user_input); // Overflow if input > 99 chars + +// FIX +char buffer[100]; +strncpy(buffer, user_input, sizeof(buffer) - 1); +buffer[sizeof(buffer) - 1] = '\0'; +``` + +## Output Format + +Provide findings as: + +``` +## Memory Safety Analysis + +### Critical Issues (must fix) +1. [file.c:123] Unchecked malloc - potential NULL dereference +2. [file.c:456] Memory leak on error path - buffer not freed +3. [file.c:789] Use after free - ptr used after free() + +### Warnings (should fix) +1. [file.c:234] strcpy used - prefer strncpy +2. [file.c:567] Missing NULL check before pointer use + +### Recommendations +1. Add cleanup label for resource management +2. Use RAII wrapper in tests +3. Run valgrind in CI pipeline + +### Suggested Fixes +[Provide specific code changes for each issue] +``` + +## Verification + +After fixes: +1. All static analysis warnings resolved +2. Valgrind shows no leaks +3. All tests pass +4. Code review by human +5. Memory footprint measured and acceptable diff --git a/.github/skills/platform-portability-checker/SKILL.md b/.github/skills/platform-portability-checker/SKILL.md new file mode 100644 index 000000000..aa9c5589b --- /dev/null +++ b/.github/skills/platform-portability-checker/SKILL.md @@ -0,0 +1,318 @@ +--- +name: platform-portability-checker +description: Verify C/C++ code is platform-independent and portable across embedded platforms. Use when reviewing code for cross-platform deployment or preparing for new hardware targets. +--- + +# Platform Portability Checker + +## Purpose + +Ensure C/C++ code is portable across different embedded platforms, architectures, and operating systems without modification. + +## When to Use + +- Reviewing new code before merge +- Porting to new hardware platform +- Preparing release for multiple architectures +- Investigating platform-specific bugs +- Refactoring legacy platform-specific code + +## Portability Checklist + +### 1. Integer Types + +**Check for**: Use of `int`, `long`, `short` without fixed sizes + +```c +// PROBLEM: Size varies by platform +int counter; // 16, 32, or 64 bits? +long timestamp; // 32 or 64 bits? +short flag; // 16 bits on most, but not guaranteed + +// FIX: Use stdint.h types +#include + +uint32_t counter; // Always 32 bits +uint64_t timestamp; // Always 64 bits +uint16_t flag; // Always 16 bits + +// For size_t operations +size_t length; // Pointer-sized unsigned +ssize_t result; // Pointer-sized signed +``` + +### 2. Pointer Assumptions + +**Check for**: Pointer arithmetic, casting, size assumptions + +```c +// PROBLEM: Assumes pointer == long +long ptr_value = (long)ptr; // Fails on 64-bit with 32-bit long + +// FIX: Use uintptr_t +#include +uintptr_t ptr_value = (uintptr_t)ptr; + +// PROBLEM: Pointer used as integer +if (ptr & 0x1) { ... } // What size is ptr? + +// FIX: Be explicit +if ((uintptr_t)ptr & 0x1) { ... } +``` + +### 3. Endianness + +**Check for**: Multi-byte values sent over network or stored to disk + +```c +// PROBLEM: Host byte order assumed +uint32_t value = 0x12345678; +fwrite(&value, 4, 1, file); // Different on LE vs BE + +// FIX: Explicit byte order +#include // For htonl, ntohl + +uint32_t host_value = 0x12345678; +uint32_t network_value = htonl(host_value); +fwrite(&network_value, 4, 1, file); + +// For reading +uint32_t network_value; +fread(&network_value, 4, 1, file); +uint32_t host_value = ntohl(network_value); +``` + +### 4. Structure Packing + +**Check for**: Structures sent over network or saved to disk + +```c +// PROBLEM: Padding varies by platform +struct { + uint8_t type; + uint32_t value; // Padding before this? + uint16_t flags; // Padding before this? +} data; + +// FIX: Explicit packing +struct __attribute__((packed)) { + uint8_t type; + uint32_t value; + uint16_t flags; +} data; + +// Or control padding explicitly +struct { + uint8_t type; + uint8_t padding[3]; // Explicit padding + uint32_t value; + uint16_t flags; + uint16_t padding2; +} data; +``` + +### 5. Boolean Type + +**Check for**: Using int/char for boolean + +```c +// PROBLEM: Non-standard boolean +int flag; // Really 3 states: 0, 1, other +char enabled; // Also used for booleans + +// FIX: Use stdbool.h +#include + +bool flag; +bool enabled; + +if (flag) { ... } // Clear intent +``` + +### 6. Character Sets + +**Check for**: Assumptions about ASCII or character encoding + +```c +// PROBLEM: Assumes ASCII +if (ch >= 'A' && ch <= 'Z') { + ch = ch + 32; // Convert to lowercase? +} + +// FIX: Use standard functions +#include + +if (isupper(ch)) { + ch = tolower(ch); +} +``` + +### 7. File Paths + +**Check for**: Hard-coded path separators + +```c +// PROBLEM: Unix-specific +const char* path = "/tmp/telemetry/data.log"; + +// FIX: Use platform-agnostic approach +#ifdef _WIN32 + #define PATH_SEP "\\" + const char* tmp_dir = getenv("TEMP"); +#else + #define PATH_SEP "/" + const char* tmp_dir = "/tmp"; +#endif + +char path[256]; +snprintf(path, sizeof(path), "%s%stelemetry%sdata.log", + tmp_dir, PATH_SEP, PATH_SEP); +``` + +### 8. System Calls + +**Check for**: Platform-specific syscalls + +```c +// PROBLEM: Linux-specific +#include +int fd = epoll_create(10); + +// FIX: Abstraction layer +// In platform.h +#if defined(__linux__) + #include "platform_linux.h" +#elif defined(__APPLE__) + #include "platform_darwin.h" +#else + #error "Unsupported platform" +#endif + +// Each platform provides same interface +event_loop_t* create_event_loop(void); +``` + +### 9. Compiler Extensions + +**Check for**: GCC/Clang specific features + +```c +// PROBLEM: GCC-specific +typeof(x) y = x; +int array[0]; // Zero-length array + +// FIX: Avoid compiler-specific typeof/__auto_type; use standard types +int y = x; // declare with explicit type + +// Or avoid non-standard features +// Define proper types instead +``` + +### 10. Include Paths + +**Check for**: Platform-specific headers + +```c +// PROBLEM: Assumes Linux headers +#include + +// FIX: Use standard headers or configure check +#ifdef HAVE_LINUX_LIMITS_H + #include +#else + #include +#endif + +// Or use autoconf to detect +// In configure.ac: +// AC_CHECK_HEADERS([linux/limits.h limits.h]) +``` + +## Build System Integration + +### configure.ac checks + +```autoconf +# Check for required features +AC_C_BIGENDIAN +AC_CHECK_SIZEOF([int]) +AC_CHECK_SIZEOF([long]) +AC_CHECK_SIZEOF([void *]) + +# Check for headers +AC_CHECK_HEADERS([stdint.h stdbool.h endian.h]) + +# Check for functions +AC_CHECK_FUNCS([htonl ntohl]) + +# Platform-specific code +case "$host" in + *-linux*) + AC_DEFINE([PLATFORM_LINUX], [1]) + ;; + arm*|*-arm*) + AC_DEFINE([PLATFORM_ARM], [1]) + ;; +esac +``` + +## Testing + +### Cross-Compilation Test + +```bash +# Test building for different architectures +./configure --host=arm-linux-gnueabihf +make clean && make + +./configure --host=x86_64-linux-gnu +make clean && make + +./configure --host=mips-linux-gnu +make clean && make +``` + +### Endianness Test + +```c +// Test endianness handling +uint32_t value = 0x12345678; +uint32_t network = htonl(value); +uint32_t restored = ntohl(network); +assert(value == restored); + +// Verify structure packing +assert(sizeof(packed_struct_t) == EXPECTED_SIZE); +``` + +## Output Format + +``` +## Platform Portability Analysis + +### Critical Issues +1. [file.c:123] Using `long` for timestamp - not fixed width +2. [file.c:456] Writing struct directly to network - endianness issue +3. [file.c:789] Assuming 32-bit pointers + +### Warnings +1. [file.c:234] Using int for boolean - prefer stdbool.h +2. [file.c:567] Hard-coded Unix path separator + +### Recommendations +1. Add configure checks for required headers +2. Create platform abstraction layer +3. Test build on multiple architectures + +### Suggested Fixes +[Specific code changes for each issue] +``` + +## Verification + +- Code compiles on target platforms +- Tests pass on all platforms +- Static analysis clean +- No endianness issues +- No alignment issues +- Structure sizes verified diff --git a/.github/skills/quality-checker/README.md b/.github/skills/quality-checker/README.md new file mode 100644 index 000000000..1d6482a0b --- /dev/null +++ b/.github/skills/quality-checker/README.md @@ -0,0 +1,72 @@ +# Quality Checker Skill + +Run comprehensive quality checks in the standard test container through chat interface. + +## Quick Start + +Simply ask Copilot to run quality checks in natural language: + +```text +Run quality checks +``` + +```text +Check memory safety +``` + +```text +Run static analysis on uploadstblogs/src +``` + +## What Gets Checked + +1. **Static Analysis**: cppcheck + shellcheck +2. **Memory Safety**: valgrind leak detection +3. **Thread Safety**: helgrind race/deadlock detection +4. **Build Verification**: strict warnings compilation + +## Environment + +Runs in the same container as CI/CD: + +- Image: `ghcr.io/rdkcentral/docker-device-mgt-service-test/native-platform:latest` +- All tools pre-installed +- Consistent with automated tests + +## Example Invocations + +| What to say | What it does | +| ----------- | ------------ | +| "Run quality checks" | Full suite, summary report | +| "Quick static analysis" | cppcheck + shellcheck only | +| "Check for memory leaks" | valgrind on test binaries | +| "Verify build with strict warnings" | Build with -Werror | +| "Run all checks on source/utils" | Full suite, scoped to utils | + +## Typical Workflow + +1. **Before committing**: "Run static analysis" +2. **Before push**: "Run quality checks" +3. **Debugging crash**: "Check memory safety" +4. **Reviewing PR**: "Run all checks" + +## Output + +You'll receive: + +- Summary of issues found +- Critical problems highlighted +- Links to detailed reports +- Recommendations for fixes + +## Prerequisites + +- Docker installed and running +- Access to GitHub Container Registry (automatic in CI/CD, may need login locally) + +## Tips + +- Start with static analysis (fastest) +- Run memory checks after static analysis passes +- Scope checks to changed files for speed +- Full suite before pushing to develop branch diff --git a/.github/skills/quality-checker/SKILL.md b/.github/skills/quality-checker/SKILL.md new file mode 100644 index 000000000..a29ebc916 --- /dev/null +++ b/.github/skills/quality-checker/SKILL.md @@ -0,0 +1,329 @@ +--- +name: quality-checker +description: Run comprehensive quality checks (static analysis, memory safety, thread safety, build verification) in the standard test container. Use when validating code changes or debugging before committing. +--- + +# Container-Based Quality Checker + +## Purpose + +Execute comprehensive quality checks on the codebase using the same containerized environment as CI/CD pipelines. Ensures consistency between local development and automated testing. + +## Usage + +Invoke this skill when: +- Validating changes before committing +- Debugging build or test failures +- Running quality checks locally +- Verifying memory safety of new code +- Checking for thread safety issues +- Performing static analysis + +You can run all checks or select specific ones based on your needs. + +## What It Does + +This skill runs quality checks inside the official test container (`ghcr.io/rdkcentral/docker-device-mgt-service-test/native-platform:latest`), which includes: +- Build tools (gcc, g++, autotools, make) +- Static analysis tools (cppcheck, shellcheck) +- Memory analysis tools (valgrind) +- Thread analysis tools (helgrind) +- Google Test/Mock frameworks + +## Available Checks + +### 1. Static Analysis +- **cppcheck**: Comprehensive C/C++ static code analyzer +- **shellcheck**: Shell script linter +- **Output**: XML report with findings + +### 2. Memory Safety (Valgrind) +- **Memory leak detection**: Finds unreleased allocations +- **Use-after-free detection**: Catches dangling pointer usage +- **Invalid memory access**: Buffer overflows, uninitialized reads +- **Output**: XML and log files per test binary + +### 3. Thread Safety (Helgrind) +- **Race condition detection**: Finds unsynchronized shared memory access +- **Deadlock detection**: Identifies lock ordering issues +- **Lock usage verification**: Validates proper synchronization +- **Output**: XML and log files per test binary + +### 4. Build Verification +- **Strict compilation**: Builds with `-Wall -Wextra -Werror` +- **Test build**: Verifies tests compile +- **Binary analysis**: Reports size and dependencies +- **Output**: Build artifacts and size report + +## Execution Process + +### Step 1: Setup Container Environment + +Pull the latest test container: +```bash +docker pull ghcr.io/rdkcentral/docker-device-mgt-service-test/native-platform:latest +``` + +Start container with workspace mounted: +```bash +docker run -d --name native-platform \ + -v /path/to/workspace:/mnt/workspace \ + ghcr.io/rdkcentral/docker-device-mgt-service-test/native-platform:latest +``` + +### Step 2: Run Selected Checks + +Execute the requested quality checks inside the container: + +**Static Analysis:** +```bash +docker exec -i native-platform /bin/bash -c " + cd /mnt/workspace && \ + cppcheck --enable=all \ + --inconclusive \ + --suppress=missingIncludeSystem \ + --suppress=unmatchedSuppression \ + --error-exitcode=0 \ + --xml \ + --xml-version=2 \ + . 2> cppcheck-report.xml +" +``` + +**Shell Script Checks:** +```bash +docker exec -i native-platform /bin/bash -c " + cd /mnt/workspace && \ + find . -name '*.sh' -type f -exec shellcheck {} + +" +``` + +**Memory Safety:** +```bash +docker exec -i native-platform /bin/bash -c " + cd /mnt/workspace && \ + autoreconf -fi && \ + ./configure --enable-gtest && \ + make -j\$(nproc) && \ + find unittest uploadstblogs/unittest -type f -executable -name '*gtest*' 2>/dev/null | while read test_bin; do + valgrind --leak-check=full \ + --show-leak-kinds=all \ + --track-origins=yes \ + --xml=yes \ + --xml-file=\"valgrind-\$(basename \$test_bin).xml\" \ + \"\$test_bin\" 2>&1 | tee \"valgrind-\$(basename \$test_bin).log\" + done +" +``` + +**Thread Safety:** +```bash +docker exec -i native-platform /bin/bash -c " + cd /mnt/workspace && \ + find unittest uploadstblogs/unittest -type f -executable -name '*gtest*' 2>/dev/null | while read test_bin; do + valgrind --tool=helgrind \ + --track-lockorders=yes \ + --xml=yes \ + --xml-file=\"helgrind-\$(basename \$test_bin).xml\" \ + \"\$test_bin\" 2>&1 | tee \"helgrind-\$(basename \$test_bin).log\" + done +" +``` + +**Build Verification:** +```bash +docker exec -i native-platform /bin/bash -c " + cd /mnt/workspace && \ + autoreconf -fi && \ + ./configure --enable-gtest CFLAGS='-Wall -Wextra -Werror' CXXFLAGS='-Wall -Wextra -Werror' && \ + make -j\$(nproc) && \ + if [ -f 'dcmd' ]; then + ls -lh dcmd + file dcmd + size dcmd + fi + if [ -f 'uploadstblogs/src/uploadstblogs' ]; then + ls -lh uploadstblogs/src/uploadstblogs + file uploadstblogs/src/uploadstblogs + fi +" +``` + +### Step 3: Report Results + +Parse and summarize results for the user: +- Number of issues found by category +- Critical issues requiring immediate attention +- Warnings that should be addressed +- Memory leaks with stack traces +- Race conditions or deadlock risks +- Build errors or warnings + +### Step 4: Cleanup + +Stop and remove the container: +```bash +docker stop native-platform +docker rm native-platform +``` + +## Interpreting Results + +### Static Analysis (cppcheck) +- **error**: Critical issues that must be fixed +- **warning**: Potential problems to review +- **style**: Code style improvements +- **performance**: Optimization opportunities + +### Memory Safety (Valgrind) +- **definitely lost**: Memory leaks requiring fixes +- **indirectly lost**: Leaks from lost parent structures +- **possibly lost**: Potential leaks to investigate +- **still reachable**: Memory held at exit (usually OK) +- **Invalid read/write**: Buffer overflow (CRITICAL) +- **Use of uninitialized value**: Must initialize before use + +### Thread Safety (Helgrind) +- **Possible data race**: Unsynchronized access to shared data +- **Lock order violation**: Potential deadlock scenario +- **Unlocking unlocked lock**: Synchronization bug +- **Thread still holds locks**: Resource leak + +### Build Verification +- **Compilation errors**: Must fix before proceeding +- **Warnings**: Review and fix (builds with -Werror) +- **Binary size**: Monitor for embedded constraints + +## User Interaction + +When invoked, ask the user: + +1. **Which checks to run?** + - All checks (comprehensive) + - Static analysis only (fast) + - Memory safety only + - Thread safety only + - Build verification only + - Custom combination + +2. **Scope:** + - Full codebase + - Specific directories + - Recently changed files + +3. **Report detail:** + - Summary only (counts and critical issues) + - Detailed (all findings) + - Full raw output + +## Example Invocations + +**User**: "Run quality checks" +- Default: Run all checks on full codebase, provide summary + +**User**: "Check memory safety" +- Run only valgrind checks, detailed report + +**User**: "Quick static analysis" +- Run cppcheck and shellcheck, summary only + +**User**: "Verify my changes build" +- Run build verification with strict warnings + +**User**: "Full analysis on uploadstblogs/src" +- Run all checks scoped to uploadstblogs directory + +## Best Practices + +1. **Run before committing**: Catch issues early +2. **Start with static analysis**: Fastest feedback +3. **Run memory checks on test binaries**: Most effective +4. **Review thread safety for concurrent code**: Essential for multi-threaded components +5. **Monitor binary size**: Important for embedded targets + +## Integration with Development Workflow + +1. **Pre-commit**: Quick static analysis +2. **Pre-push**: Full quality check suite +3. **Debugging**: Targeted memory/thread analysis +4. **Code review**: Validate reviewer feedback +5. **Refactoring**: Ensure no regressions + +## Advantages Over Manual Testing + +- **Consistency**: Same environment as CI/CD +- **Completeness**: All tools in one command +- **Reproducibility**: Container ensures identical results +- **Efficiency**: No local tool installation needed +- **Confidence**: Pass locally = pass in CI + +## Output Files Generated + +- `cppcheck-report.xml`: Static analysis findings +- `valgrind-.xml`: Memory issues per test +- `valgrind-.log`: Detailed memory logs +- `helgrind-.xml`: Thread safety issues per test +- `helgrind-.log`: Detailed concurrency logs + +These files can be uploaded as artifacts or reviewed locally. + +## Limitations + +- Requires Docker with GitHub Container Registry access +- Container pulls can be slow on first run (cached afterward) +- Full suite can take several minutes depending on codebase size +- Valgrind slows execution significantly (expected) + +## Tips for Faster Execution + +1. Use cached container images (don't pull every time) +2. Run static analysis first (fastest) +3. Scope checks to changed directories +4. Run memory/thread checks only on affected tests +5. Use parallel execution where possible + +## Skill Execution Logic + +When user invokes this skill: + +1. **Authenticate with GitHub Container Registry** + - Use github.actor and GITHUB_TOKEN if available + - Otherwise prompt for credentials or skip private registries + +2. **Pull container image** + - Check if image exists locally + - Pull only if needed or if --force specified + +3. **Start container** + - Mount workspace at /mnt/workspace + - Use unique container name (quality-checker-) + - Run in detached mode + +4. **Execute requested checks** + - Run checks in sequence + - Capture output + - Continue on errors (collect all findings) + +5. **Collect results** + - Copy result files from container + - Parse XML/log outputs + - Categorize findings + +6. **Report to user** + - Summary count + - Critical issues highlighted + - Link to detailed reports + - Next steps recommendations + +7. **Cleanup** + - Stop container + - Remove container + - Optional: clean up result files + +## Error Handling + +- **Container pull fails**: Report error, suggest manual pull +- **Container start fails**: Check Docker daemon, ports, permissions +- **Build fails**: Report build errors, stop further checks +- **Tools missing**: Verify container version, report missing tools +- **Out of memory**: Suggest increasing Docker memory limit diff --git a/.github/skills/technical-documentation-writer/SKILL.md b/.github/skills/technical-documentation-writer/SKILL.md new file mode 100644 index 000000000..bd9cff1a1 --- /dev/null +++ b/.github/skills/technical-documentation-writer/SKILL.md @@ -0,0 +1,712 @@ +--- +name: technical-documentation-writer +description: Create and maintain comprehensive technical documentation for embedded systems projects. Use for architecture docs, API references, developer guides, and system documentation following best practices. +--- + +# Technical Documentation Writer for Embedded Systems + +## Purpose + +Create clear, comprehensive, and maintainable technical documentation for embedded C/C++ projects, with focus on architecture, APIs, threading models, memory management, and platform integration. + +## Usage + +Invoke this skill when: +- Documenting new features or components +- Creating system architecture documentation +- Writing API reference documentation +- Documenting threading and synchronization models +- Creating developer onboarding guides +- Documenting debugging procedures +- Writing integration guides for platform vendors + +## Documentation Structure + +### Directory Layout + +``` +project/ +├── README.md # Project overview, quick start +├── docs/ # General documentation +│ ├── README.md # Documentation index +│ ├── architecture/ # System architecture +│ │ ├── overview.md # High-level architecture +│ │ ├── component-diagram.md # Component relationships +│ │ ├── threading-model.md # Threading architecture +│ │ └── data-flow.md # Data flow diagrams +│ ├── api/ # API documentation +│ │ ├── public-api.md # Public API reference +│ │ └── internal-api.md # Internal API reference +│ ├── integration/ # Integration guides +│ │ ├── build-setup.md # Build environment setup +│ │ ├── platform-porting.md # Porting to new platforms +│ │ └── testing.md # Test procedures +│ └── troubleshooting/ # Debug guides +│ ├── memory-issues.md # Memory debugging +│ ├── threading-issues.md # Thread debugging +│ └── common-errors.md # Common error solutions +└── source/ # Source code + └── docs/ # Component-specific docs + ├── bulkdata/ # Mirrors source structure + │ ├── README.md # Component overview + │ └── profile-management.md + ├── protocol/ + │ ├── README.md + │ └── http-architecture.md + └── scheduler/ + ├── README.md + └── scheduling-algorithm.md +``` + +### Document Types + +#### 1. **Architecture Documentation** (`docs/architecture/`) +- System overview and design principles +- Component relationships and dependencies +- Threading and concurrency models +- Data flow and state machines +- Memory management strategies +- Platform abstraction layers + +#### 2. **API Documentation** (`docs/api/`) +- Public API reference with examples +- Internal API documentation +- Function contracts and preconditions +- Thread-safety guarantees +- Memory ownership semantics +- Error handling conventions + +#### 3. **Component Documentation** (`source/docs/`) +- Per-component technical details +- Algorithm explanations +- Implementation notes +- Performance characteristics +- Resource usage (memory, CPU, threads) +- Dependencies and interfaces + +#### 4. **Integration Guides** (`docs/integration/`) +- Build system setup +- Platform porting guides +- Configuration options +- Testing procedures +- Deployment checklists + +#### 5. **Troubleshooting Guides** (`docs/troubleshooting/`) +- Common error scenarios +- Debug techniques +- Log analysis +- Memory profiling +- Thread race detection + +## Documentation Process + +### Step 1: Analyze the Code + +Before writing documentation: + +1. **Read the source code** - Understand implementation +2. **Identify key abstractions** - Classes, structs, modules +3. **Map dependencies** - What calls what, data flow +4. **Find synchronization** - Mutexes, conditions, atomics +5. **Trace resource lifecycle** - Allocations, ownership, cleanup +6. **Review existing docs** - Check for patterns and style + +### Step 2: Create Structure + +For each component: + +```markdown +# Component Name + +## Overview +Brief 2-3 sentence description of purpose and role. + +## Architecture +High-level design with diagrams. + +## Key Components +List main structures, functions, modules. + +## Threading Model +How threads interact, synchronization primitives. + +## Memory Management +Allocation patterns, ownership, lifecycle. + +## API Reference +Public functions with signatures and examples. + +## Usage Examples +Common use cases with code snippets. + +## Error Handling +Error codes, failure modes, recovery. + +## Performance Considerations +Resource usage, bottlenecks, optimization tips. + +## Platform Notes +Platform-specific behavior or requirements. + +## Testing +How to test, test coverage, known issues. + +## See Also +Cross-references to related documentation. +``` + +### Step 3: Add Diagrams + +Use Mermaid for visual documentation: + +#### Component Diagram +```mermaid +graph TB + A[Client] --> B[Connection Pool] + B --> C[CURL Handle 1] + B --> D[CURL Handle 2] + B --> E[CURL Handle N] + C --> F[libcurl] + D --> F + E --> F + F --> G[HTTP Server] +``` + +#### Sequence Diagram +```mermaid +sequenceDiagram + participant Client + participant Pool + participant CURL + participant Server + + Client->>Pool: Request handle + Pool->>Pool: Lock mutex + Pool-->>Client: Return handle + Client->>CURL: Configure request + Client->>CURL: Execute + CURL->>Server: HTTP Request + Server-->>CURL: Response + CURL-->>Client: Result + Client->>Pool: Release handle + Pool->>Pool: Signal condition +``` + +#### State Diagram +```mermaid +stateDiagram-v2 + [*] --> Uninitialized + Uninitialized --> Initialized: init() + Initialized --> Running: start() + Running --> Paused: pause() + Paused --> Running: resume() + Running --> Stopped: stop() + Stopped --> [*] +``` + +#### Data Flow Diagram +```mermaid +flowchart LR + A[Marker Event] --> B{Event Type} + B -->|Component| C[Component Marker] + B -->|Event| D[Event Marker] + C --> E[Profile Matcher] + D --> E + E --> F[Report Generator] + F --> G[HTTP Sender] +``` + +### Step 4: Add Code Examples + +Provide clear, compilable examples: + +#### Good Example Structure +```markdown +### Example: Creating a Profile + +This example shows how to create and configure a telemetry profile. + +**Prerequisites:** +- Telemetry system initialized +- Valid configuration file + +**Code:** +```c +#include "profile.h" +#include + +int main(void) { + profile_t* profile = NULL; + int ret = 0; + + // Create profile with name and interval + ret = profile_create("MyProfile", 60, &profile); + if (ret != 0) { + fprintf(stderr, "Failed to create profile: %d\n", ret); + return -1; + } + + // Add marker to profile + ret = profile_add_marker(profile, "Component.Status", + MARKER_TYPE_COMPONENT); + if (ret != 0) { + fprintf(stderr, "Failed to add marker: %d\n", ret); + profile_destroy(profile); + return -1; + } + + // Activate profile + ret = profile_activate(profile); + if (ret != 0) { + fprintf(stderr, "Failed to activate profile: %d\n", ret); + profile_destroy(profile); + return -1; + } + + printf("Profile created and activated successfully\n"); + + // Cleanup + profile_destroy(profile); + return 0; +} +``` + +**Expected Output:** +``` +Profile created and activated successfully +``` + +**Notes:** +- Always check return values +- Call profile_destroy() even on error paths +- Profile name must be unique + +### Step 5: Document APIs + +For each public function: + +```markdown +### profile_create() + +Creates a new telemetry profile. + +**Signature:** +```c +int profile_create(const char* name, + unsigned int interval_sec, + profile_t** out_profile); +``` + +**Parameters:** +- `name` - Unique profile name (max 63 chars, non-NULL) +- `interval_sec` - Reporting interval in seconds (min: 60, max: 86400) +- `out_profile` - Output pointer to created profile (must be non-NULL) + +**Returns:** +- `0` - Success +- `-EINVAL` - Invalid parameter (NULL name/out_profile, invalid interval) +- `-ENOMEM` - Memory allocation failed +- `-EEXIST` - Profile with same name already exists + +**Thread Safety:** +Thread-safe. Uses internal mutex for profile list management. + +**Memory:** +Allocates memory for profile structure and name copy. Caller must call +`profile_destroy()` to free resources. + +**Example:** +See [Example: Creating a Profile](#example-creating-a-profile) + +**See Also:** +- profile_destroy() +- profile_activate() +- profile_add_marker() +``` + +### Step 6: Document Threading + +For multi-threaded components: + +```markdown +## Threading Model + +### Thread Overview + +| Thread Name | Purpose | Priority | Stack Size | +|------------|---------|----------|------------| +| Main | Initialization, message loop | Normal | Default | +| XConf Fetch | Configuration retrieval | Low | 64KB | +| Report Send | HTTP report transmission | Low | 64KB | +| Event Receiver | Marker event processing | High | 32KB | + +### Synchronization Primitives + +```c +// Global mutexes +static pthread_mutex_t pool_mutex = PTHREAD_MUTEX_INITIALIZER; +static pthread_mutex_t profile_mutex = PTHREAD_MUTEX_INITIALIZER; + +// Condition variables +static pthread_cond_t pool_cond = PTHREAD_COND_INITIALIZER; +static pthread_cond_t xconf_cond = PTHREAD_COND_INITIALIZER; +``` + +### Lock Ordering + +To prevent deadlocks, always acquire locks in this order: + +1. `profile_mutex` (profile list) +2. `pool_mutex` (connection pool) +3. Individual profile locks + +**Example:** +```c +// CORRECT: Proper lock ordering +pthread_mutex_lock(&profile_mutex); +profile_t* p = find_profile_locked(name); +pthread_mutex_lock(&pool_mutex); +// ... use both resources ... +pthread_mutex_unlock(&pool_mutex); +pthread_mutex_unlock(&profile_mutex); + +// WRONG: Deadlock risk! +pthread_mutex_lock(&pool_mutex); +pthread_mutex_lock(&profile_mutex); // May deadlock! +``` + +### Thread Safety Guarantees + +| Function | Thread Safety | Notes | +|----------|---------------|-------| +| profile_create() | Thread-safe | Uses profile_mutex | +| profile_destroy() | Thread-safe | Uses profile_mutex | +| profile_add_marker() | Not thread-safe | Call before activation only | +| send_report() | Thread-safe | Uses pool_mutex | +``` + +### Step 7: Document Memory Management + +```markdown +## Memory Management + +### Allocation Patterns + +```mermaid +graph TD + A[profile_create] --> B[malloc profile_t] + B --> C[strdup name] + B --> D[malloc markers array] + E[profile_add_marker] --> F[realloc markers] + G[profile_destroy] --> H[free markers] + H --> I[free name] + I --> J[free profile_t] +``` + +### Ownership Rules + +1. **profile_t**: Owned by caller after profile_create() +2. **Marker strings**: Copied; caller retains original ownership +3. **Report data**: Owned by sender; freed after transmission + +### Lifecycle Example + +```c +// Creation phase +profile_t* prof = NULL; +profile_create("test", 60, &prof); // Allocates memory + +// Configuration phase +profile_add_marker(prof, "mark1", TYPE_EVENT); // May realloc +profile_add_marker(prof, "mark2", TYPE_EVENT); // May realloc + +// Active phase - no allocations +profile_activate(prof); + +// Destruction phase +profile_destroy(prof); // Frees all memory +prof = NULL; // Prevent use-after-free +``` + +### Memory Budget + +Typical memory usage per component: + +| Component | Static | Dynamic (per item) | Notes | +|-----------|--------|-------------------|-------| +| Profile | 128 bytes | +32 bytes/marker | Preallocated list | +| Connection Pool | 512 bytes | +256 bytes/handle | Max 5 handles | +| Report Buffer | 0 | 64KB | Temporary, freed after send | + +**Total typical footprint**: ~150KB (5 profiles, 3 connections, 1 report) +``` + +## Best Practices + +### Writing Style + +1. **Be Concise**: Get to the point quickly +2. **Be Specific**: Use exact terms, not vague descriptions +3. **Be Accurate**: Test all code examples +4. **Be Complete**: Don't leave critical details unstated +5. **Be Consistent**: Follow established patterns + +### Code Examples + +- **Always compile-test** examples before documenting +- **Show error handling** - embedded systems need robust code +- **Include cleanup** - demonstrate proper resource management +- **Add context** - explain when/why to use the code +- **Keep focused** - one example, one concept + +### Diagrams + +- **Use Mermaid** for all diagrams (version control friendly) +- **Keep simple** - max 10-12 nodes per diagram +- **Label clearly** - all arrows and nodes need names +- **Show flow** - make direction obvious +- **Add legends** - explain symbols if needed + +### Cross-References + +Link related documentation: + +```markdown +## See Also + +- [Threading Model](../architecture/threading-model.md) - Overall thread architecture +- [Connection Pool API](connection-pool.md) - Pool management functions +- [Error Codes](../api/error-codes.md) - Complete error code reference +- [Build Guide](../integration/build-setup.md) - Compilation instructions +``` + +### Platform-Specific Notes + +Always document platform variations: + +```markdown +## Platform Notes + +### Linux +- Uses pthread for threading +- Requires libcurl 7.65.0+ +- mTLS via OpenSSL 1.1.1+ + +### RDKB Devices +- Integration with RDK logger (rdk_debug.h) +- Uses RBUS for IPC when available +- Memory constraints: limit to 8 profiles max + +### Constraints +- **Memory**: Tested with 64MB minimum +- **CPU**: ARMv7 or better +- **Storage**: 1MB for logs and cache +``` + +## Output Format + +### Component Documentation Template + +```markdown +# [Component Name] + +## Overview + +[2-3 sentence description] + +## Architecture + +[High-level design explanation] + +### Component Diagram +```mermaid +[Component relationship diagram] +``` + +## Key Components + +### [Structure/Type Name] + +[Description] + +```c +typedef struct { + // Fields with comments +} structure_t; +``` + +## Threading Model + +[Thread safety and synchronization] + +## Memory Management + +[Allocation patterns and ownership] + +## API Reference + +### [function_name()] + +[Full API documentation] + +## Usage Examples + +### Example: [Use Case] + +[Complete working example] + +## Error Handling + +[Error codes and recovery] + +## Performance + +[Resource usage and bottlenecks] + +## Testing + +[Test procedures and coverage] + +## See Also + +[Cross-references] +``` + +## Quality Checklist + +Before considering documentation complete: + +- [ ] All public APIs documented with signatures +- [ ] At least one working code example per major function +- [ ] Thread safety explicitly stated +- [ ] Memory ownership clearly documented +- [ ] Error codes and meanings listed +- [ ] Diagrams for complex flows +- [ ] Cross-references to related docs +- [ ] Platform-specific notes included +- [ ] Code examples compile and run +- [ ] Grammar and spelling checked +- [ ] Reviewed by component author + +## Maintenance + +Documentation is code: + +1. **Update with code changes** - docs and code change together +2. **Version documentation** - tag with releases +3. **Review periodically** - ensure accuracy quarterly +4. **Fix broken links** - validate references +5. **Deprecate carefully** - mark old features clearly + +### Deprecation Notice Template + +```markdown +## DEPRECATED: old_function() + +⚠️ **This function is deprecated as of v2.1.0** + +**Reason**: Memory leak risk in error paths + +**Alternative**: Use new_function() instead + +**Migration Example**: +```c +// Old way (deprecated) +old_function(param); + +// New way +new_function(param); +``` + +**Removal**: Scheduled for v3.0.0 (Est. Q2 2026) +``` + +## Tools Integration + +### Generate API Docs from Code + +Use Doxygen-style comments in code: + +```c +/** + * @brief Create a new telemetry profile + * + * Creates and initializes a profile structure. The caller is responsible + * for destroying the profile with profile_destroy() when done. + * + * @param[in] name Unique profile name (max 63 chars) + * @param[in] interval_sec Reporting interval (60-86400 seconds) + * @param[out] out_profile Pointer to receive created profile + * + * @return 0 on success, negative errno on failure + * @retval 0 Success + * @retval -EINVAL Invalid parameter + * @retval -ENOMEM Memory allocation failed + * @retval -EEXIST Profile already exists + * + * @note Thread-safe + * @see profile_destroy(), profile_activate() + * + * @par Example: + * @code + * profile_t* prof = NULL; + * int ret = profile_create("MyProfile", 300, &prof); + * if (ret == 0) { + * // Use profile... + * profile_destroy(prof); + * } + * @endcode + */ +int profile_create(const char* name, + unsigned int interval_sec, + profile_t** out_profile); +``` + +### Diagram Tools + +- **Mermaid Live Editor**: https://mermaid.live +- **VS Code Markdown Preview**: Built-in mermaid support +- **Documentation generators**: Can embed mermaid in output + +## Troubleshooting Common Documentation Issues + +### Issue: Code example doesn't compile + +**Solution**: Always test examples in isolation +```bash +# Extract example to test file +cat > test_example.c << 'EOF' +[paste example code] +EOF + +# Compile with project flags +gcc -Wall -Wextra -I../include test_example.c -o test_example + +# Run to verify +./test_example +``` + +### Issue: Diagram is too complex + +**Solution**: Break into multiple diagrams +- One high-level overview diagram +- Multiple focused detail diagrams +- Link them together in text + +### Issue: Outdated documentation + +**Solution**: Add CI check +```bash +# Check for TODOs in docs +grep -r "TODO\|FIXME\|XXX" docs/ && exit 1 + +# Check for broken links +markdown-link-check docs/**/*.md +``` + +## Example References + +See documentation references for guidance: +- [CURL Architecture](https://curl.se/docs/architecture.html) - Good example of architecture documentation with diagrams +- [Memory Safety Skill](../memory-safety-analyzer/SKILL.md) - Example skill documentation +- [Build Instructions](../../../.github/instructions/build-system.instructions.md) - Integration guide example diff --git a/.github/skills/thread-safety-analyzer/SKILL.md b/.github/skills/thread-safety-analyzer/SKILL.md new file mode 100644 index 000000000..9d413f012 --- /dev/null +++ b/.github/skills/thread-safety-analyzer/SKILL.md @@ -0,0 +1,436 @@ +--- +name: thread-safety-analyzer +description: Analyze C/C++ code for thread safety issues including race conditions, deadlocks, and improper synchronization. Use when reviewing concurrent code or debugging threading issues. +--- + +# Thread Safety Analysis for Embedded C + +## Purpose + +Systematically analyze C/C++ code for thread safety issues that can cause race conditions, deadlocks, or performance degradation in embedded systems. + +## Usage + +Invoke this skill when: +- Reviewing multi-threaded code +- Debugging race conditions or deadlocks +- Optimizing synchronization overhead +- Validating thread creation and cleanup +- Investigating lock contention issues + +## Analysis Process + +### Step 1: Identify Shared Data + +Search for global and static variables: +- Global variables (especially non-const) +- Static variables in functions +- Shared heap allocations +- Reference-counted objects + +For each shared variable, verify: +1. How is it protected (mutex, atomic, etc.)? +2. Is the protection consistent across all accesses? +3. Are reads and writes both protected? +4. Is initialization thread-safe? + +### Step 2: Review Thread Creation + +Check all pthread_create calls: +- Are thread attributes used? +- Is stack size specified? +- Are threads detached or joinable? +- Is cleanup properly handled? + +```c +// CHECK FOR: +pthread_t thread; +pthread_create(&thread, NULL, func, arg); // BAD: No attributes + +// SHOULD BE: +pthread_attr_t attr; +pthread_attr_init(&attr); +pthread_attr_setstacksize(&attr, 64 * 1024); // Explicit size +pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED); +pthread_create(&thread, &attr, func, arg); +pthread_attr_destroy(&attr); +``` + +### Step 3: Analyze Lock Usage + +For each mutex/rwlock: +- Is it initialized before use? +- Is it destroyed when done? +- Are lock/unlock pairs balanced? +- What is the lock ordering? +- Are locks held during expensive operations? + +Common patterns to check: +```c +// Pattern 1: Missing unlock on error path +pthread_mutex_lock(&lock); +if (error) return -1; // LEAK! +pthread_mutex_unlock(&lock); + +// Pattern 2: Lock ordering violation +// Thread 1: +pthread_mutex_lock(&a); +pthread_mutex_lock(&b); + +// Thread 2: +pthread_mutex_lock(&b); // Different order! +pthread_mutex_lock(&a); // DEADLOCK RISK! + +// Pattern 3: Heavy lock for simple operation +pthread_rwlock_wrlock(&lock); // Too heavy +counter++; +pthread_rwlock_unlock(&lock); +// Should use atomic_int instead +``` + +### Step 4: Check for Race Conditions + +Look for unprotected accesses to shared data: + +```c +// RACE: Read-modify-write without protection +if (shared_flag == 0) { // Thread 1 reads + shared_flag = 1; // Thread 2 also reads before Thread 1 writes +} + +// FIX: Use atomic or lock +pthread_mutex_lock(&lock); +if (shared_flag == 0) { + shared_flag = 1; +} +pthread_mutex_unlock(&lock); + +// OR: Use atomic compare-and-swap +int expected = 0; +atomic_compare_exchange_strong(&shared_flag, &expected, 1); +``` + +### Step 5: Verify Atomic Usage + +For atomic variables: +- Are they declared with proper type (atomic_int, atomic_bool)? +- Is memory ordering appropriate? +- Are non-atomic operations mixed with atomic ones? + +```c +// CHECK: +atomic_int counter; + +// GOOD: Atomic operations +atomic_fetch_add(&counter, 1); +int value = atomic_load(&counter); + +// BAD: Mixing atomic and non-atomic +counter++; // Non-atomic! Use atomic_fetch_add +``` + +### Step 6: Deadlock Detection + +Check for common deadlock patterns: + +1. **Circular wait**: Lock A → Lock B, Lock B → Lock A +2. **Lock held while waiting**: Mutex held during sleep/wait +3. **Missing timeout**: Indefinite blocking without timeout +4. **Signal under lock**: Condition signal while holding mutex + +```c +// Deadlock Pattern 1: Circular dependency +// Function 1: +lock(mutex_a); +lock(mutex_b); // Order: A, B + +// Function 2: +lock(mutex_b); +lock(mutex_a); // Order: B, A - DEADLOCK! + +// Deadlock Pattern 2: Lock held during expensive operation +lock(mutex); +expensive_network_call(); // Blocks other threads! +unlock(mutex); + +// Deadlock Pattern 3: No timeout +pthread_mutex_lock(&lock); // Waits forever if deadlock +``` + +### Step 7: Check Condition Variables + +For condition variables: +- Is wait always in a loop? +- Is predicate checked before and after wait? +- Is signal/broadcast done correctly? +- Is spurious wakeup handled? + +```c +// GOOD: Proper condition variable usage +pthread_mutex_lock(&mutex); +while (!condition) { // Loop for spurious wakeups + pthread_cond_wait(&cond, &mutex); +} +// ... use protected data ... +pthread_mutex_unlock(&mutex); + +// Signal: +pthread_mutex_lock(&mutex); +condition = true; +pthread_cond_signal(&cond); +pthread_mutex_unlock(&mutex); + +// BAD: Missing loop +pthread_mutex_lock(&mutex); +if (!condition) { // Should be 'while'! + pthread_cond_wait(&cond, &mutex); +} +pthread_mutex_unlock(&mutex); +``` + +## Common Issues and Fixes + +### Issue: Default Thread Stack Size + +```c +// PROBLEM: Wastes memory (8MB per thread) +pthread_t thread; +pthread_create(&thread, NULL, worker, arg); + +// FIX: Specify minimal stack size +pthread_attr_t attr; +pthread_attr_init(&attr); +pthread_attr_setstacksize(&attr, 64 * 1024); // 64KB +pthread_create(&thread, &attr, worker, arg); +pthread_attr_destroy(&attr); +``` + +### Issue: Heavy Synchronization + +```c +// PROBLEM: Reader-writer lock overkill +pthread_rwlock_t lock; +int counter; + +void increment() { + pthread_rwlock_wrlock(&lock); + counter++; + pthread_rwlock_unlock(&lock); +} + +// FIX: Use atomic operations +atomic_int counter; + +void increment() { + atomic_fetch_add(&counter, 1); // No lock needed +} +``` + +### Issue: Lock Ordering Violation + +```c +// PROBLEM: Different lock orders cause deadlock +// Thread 1: +void process_a_then_b() { + lock(&resource_a.lock); + lock(&resource_b.lock); + // ... +} + +// Thread 2: +void process_b_then_a() { + lock(&resource_b.lock); + lock(&resource_a.lock); // DEADLOCK! + // ... +} + +// FIX: Consistent ordering everywhere +void process_a_then_b() { + lock(&resource_a.lock); // Always A first + lock(&resource_b.lock); // Then B + // ... +} + +void process_b_then_a() { + lock(&resource_a.lock); // Always A first + lock(&resource_b.lock); // Then B + // ... +} +``` + +### Issue: Race in Lazy Initialization + +```c +// PROBLEM: Non-thread-safe initialization +static config_t* config = NULL; + +config_t* get_config() { + if (!config) { // Race here! + config = malloc(sizeof(config_t)); + init_config(config); + } + return config; +} + +// FIX: Use pthread_once +static pthread_once_t init_once = PTHREAD_ONCE_INIT; +static config_t* config = NULL; + +static void init_config_once() { + config = malloc(sizeof(config_t)); + init_config(config); +} + +config_t* get_config() { + pthread_once(&init_once, init_config_once); + return config; +} +``` + +### Issue: Missing Lock on Error Path + +```c +// PROBLEM: Lock not released on error +int process_data(data_t* shared) { + pthread_mutex_lock(&shared->lock); + + if (validate(shared) != 0) { + return -1; // BUG: Lock not released! + } + + update(shared); + pthread_mutex_unlock(&shared->lock); + return 0; +} + +// FIX: Unlock on all paths +int process_data(data_t* shared) { + int ret = 0; + + pthread_mutex_lock(&shared->lock); + + if (validate(shared) != 0) { + ret = -1; + goto cleanup; + } + + update(shared); + +cleanup: + pthread_mutex_unlock(&shared->lock); + return ret; +} +``` + +### Issue: Long Critical Section + +```c +// PROBLEM: Expensive operation under lock +pthread_mutex_lock(&lock); +for (int i = 0; i < 1000000; i++) { + compute(); // Blocks other threads! +} +shared_result = final_value; +pthread_mutex_unlock(&lock); + +// FIX: Minimize critical section +int result = 0; +for (int i = 0; i < 1000000; i++) { + result += compute(); // No lock +} + +pthread_mutex_lock(&lock); +shared_result = result; // Lock only for update +pthread_mutex_unlock(&lock); +``` + +## Testing for Thread Safety + +### Compile with Thread Sanitizer + +```bash +# Build with thread sanitizer +gcc -g -fsanitize=thread -O1 source.c -o program -lpthread + +# Run +./program + +# Will report: +# - Data races +# - Lock ordering issues +# - Potential deadlocks +``` + +### Run Helgrind + +```bash +# Check for thread safety issues +valgrind --tool=helgrind \ + --track-lockorders=yes \ + ./program + +# Reports: +# - Race conditions +# - Lock order violations +# - Possible deadlocks +``` + +### Stress Testing + +```c +// Test under high concurrency +#define NUM_THREADS 100 +#define ITERATIONS 10000 + +void stress_test() { + pthread_t threads[NUM_THREADS]; + + for (int i = 0; i < NUM_THREADS; i++) { + pthread_create(&threads[i], NULL, worker, NULL); + } + + for (int i = 0; i < NUM_THREADS; i++) { + pthread_join(threads[i], NULL); + } + + // Verify invariants + assert(shared_counter == NUM_THREADS * ITERATIONS); +} +``` + +## Output Format + +Provide findings as: + +``` +## Thread Safety Analysis + +### Critical Issues (must fix) +1. [file.c:123] Race condition - unprotected access to shared_flag +2. [file.c:456] Deadlock potential - lock order violation (A→B vs B→A) +3. [file.c:789] Lock leak - mutex not released on error path + +### Warnings (should fix) +1. [file.c:234] Default thread stack - wastes 8MB per thread +2. [file.c:567] Heavy lock - use atomic_int instead of mutex +3. [file.c:890] Long critical section - holds lock during I/O + +### Recommendations +1. Establish lock ordering convention (document in header) +2. Use pthread_once for singleton initialization +3. Replace reader-writer locks with atomics for counters +4. Add thread sanitizer to CI pipeline + +### Suggested Fixes +[Provide specific code changes for each issue] +``` + +## Verification + +After fixes: +1. Thread sanitizer shows no errors +2. Helgrind reports clean +3. Stress tests pass consistently +4. Lock contention metrics acceptable +5. No deadlocks under load testing +6. Code review confirms thread safety diff --git a/.github/skills/triage-logs/SKILL.md b/.github/skills/triage-logs/SKILL.md new file mode 100644 index 000000000..2456001e2 --- /dev/null +++ b/.github/skills/triage-logs/SKILL.md @@ -0,0 +1,398 @@ +--- +name: triage-logs +description: > + Triage any dcm-agent behavioral issue on RDK devices by correlating device + log bundles with source code. Covers daemon hangs, log upload failures, + DCM configuration errors, uploadSTBLogs failures, backup_logs issues, + USB log upload problems, RBUS communication errors, and cron scheduling + issues. The user states the issue; this skill guides systematic root-cause + analysis regardless of issue type. +--- + +# Log Triage Skill + +## Purpose + +Systematically correlate device log bundles with dcm-agent source code to +identify root causes, characterize impact, and propose unit-test and +functional-test reproduction scenarios — for **any** behavioral anomaly reported +by the user. + +--- + +## Usage + +Invoke this skill when: +- A device log bundle is available under `logs/` (or attached separately) +- The user describes a behavioral anomaly (examples: DCM daemon not starting, + log upload failures, configuration parsing errors, upload retry loops, + authentication failures, backup logs not working, USB upload issues, cron + job scheduling problems, RBUS communication failures) +- You need to write a reproduction scenario for an existing or proposed fix + +**The user's stated issue drives the investigation.** Do not assume a specific +failure mode — read the issue description first, then follow the steps below. + +--- + +## Step 1: Orient to the Log Bundle + +**Log bundle layout** (typical RDK device): +``` +logs///logs/ + dcm.log.0 ← Primary DCM daemon log (start here) + uploadstblogs.log.0 ← uploadSTBLogs log upload execution + dcmscript.log.0 ← DCM script execution logs + backup_logs.log.0 ← Log backup operations + usb_logupload.log.0 ← USB log upload operations + messages.txt.0 ← System messages + top_log.txt.0 ← CPU/memory snapshots + /opt/logs/ ← Actual log files being uploaded + /nvram/DCMresponse.txt ← DCM configuration from XConf + /nvram/dcm.properties ← DCM settings +``` + +Include any log files surfaced by the user's issue description. + +**Log timestamp prefix format**: `YYMMDD-HH:MM:SS` or RFC3339 +- Session folder names are **local-time snapshots** (format: `MM-DD-YY-HH:MMxM`) +- Log lines use device local time + +--- + +## Step 2: Map Daemon Startup and Components + +Read the startup section of `dcm.log.0` (first ~50 lines) to identify: + +| What to find | Log pattern | +|---|---| +| Daemon start | `DCM daemon starting` or `dcmDaemonMainInit` | +| Configuration loaded | `DCMresponse.txt` parsing | +| RBUS initialization | `rbus_open` or `RBUS_Initialize` | +| Cron job scheduling | `dcm_cronparse` or cron expression parsing | +| Log upload schedule | `DCM_LOG_UPLOAD` schedule setup | +| Firmware update schedule | `DCM_FW_UPDATE` schedule setup | + +**Key components in dcm-agent**: +- Main daemon (`dcmd`) — initialization, configuration, RBUS, cron scheduling +- uploadSTBLogs — log collection, archiving, upload execution +- uploadLogsNow — on-demand log upload trigger +- backup_logs — log backup and rotation +- usbLogUpload — USB-based log upload +- dcm_rbus — RBUS interface for remote control + +--- + +## Step 3: Identify the Anomaly Window + +Based on the **user's stated issue**, search for the relevant evidence pattern: + +### DCM Daemon Not Starting / Crashes +```bash +grep -n "dcmDaemonMainInit\|ERROR\|FATAL\|Segmentation\|core dump" dcm.log.0 +grep -n "dcmd" messages.txt.0 | tail -50 +``` +Check for: +- Configuration file missing or malformed (`/nvram/DCMresponse.txt`) +- RBUS initialization failure +- Memory allocation failures +- Dependency library missing (rbus, curl, ssl) + +### Log Upload Failures +```bash +grep -n "uploadSTBLogs\|upload\|ERROR\|HTTP\|curl\|Failed" uploadstblogs.log.0 +grep -n "S3\|presign\|mTLS\|OAuth\|authentication" uploadstblogs.log.0 +``` +Look for: +- HTTP status codes (4xx client errors, 5xx server errors) +- Curl error codes +- Authentication failures (certificate errors, OAuth token issues) +- Pre-sign request failures +- Network connectivity issues +- Retry exhaustion + +### Configuration Parsing Errors +```bash +grep -n "dcm_parseconf\|parse\|ERROR\|Invalid" dcm.log.0 +cat /nvram/DCMresponse.txt # Check configuration format +``` +Verify: +- JSON/XML syntax validity +- Required fields present (URL, schedule) +- Upload protocol configuration (HTTP, HTTPS) +- Authentication settings + +### Cron Scheduling Issues +```bash +grep -n "dcm_cronparse\|cron\|schedule\|ERROR" dcm.log.0 +``` +Check: +- Cron expression validity +- Schedule parsing errors +- Job execution timing +- Missed schedule windows + +### RBUS Communication Errors +```bash +grep -n "rbus\|RBUS_ERROR\|connection\|method" dcm.log.0 +``` +Verify: +- RBUS daemon (rtrouted) running +- Method registration success +- Event subscription success +- Method invocation errors + +### Upload Strategy Issues +```bash +grep -n "strategy\|RRD\|OnDemand\|Reboot\|DCM\|Non-DCM" uploadstblogs.log.0 +``` +Identify: +- Which strategy was selected +- Strategy selection logic +- Trigger conditions met/not met +- Early abort conditions (privacy mode, no logs) + +### Archive/Packaging Failures +```bash +grep -n "archive\|tar\|gzip\|packaging\|collection" uploadstblogs.log.0 +``` +Check for: +- Disk space issues +- File permission errors +- Tar/gzip failures +- Log file collection errors + +--- + +## Step 4: Correlate with Source Code + +Map log evidence to source files: + +| Issue Area | Source Files | +|---|---| +| Daemon initialization | `dcm.c`, `dcm_parseconf.c` | +| RBUS interface | `dcm_rbus.c` | +| Cron parsing | `dcm_cronparse.c` | +| Job scheduling | `dcm_schedjob.c` | +| Configuration parsing | `dcm_parseconf.c` | +| uploadSTBLogs main logic | `uploadstblogs/src/uploadstblogs.c` | +| Upload strategies | `uploadstblogs/src/strategy_*.c`, `uploadstblogs/src/strategy_selector.c` | +| Upload engine | `uploadstblogs/src/upload_engine.c` | +| Retry logic | `uploadstblogs/src/retry_logic.c` | +| Archive management | `uploadstblogs/src/archive_manager.c` | +| Authentication | `uploadstblogs/src/` (mTLS/OAuth handling) | +| RBUS interface | `uploadstblogs/src/rbus_interface.c` | +| On-demand upload | `uploadstblogs/src/uploadlogsnow.c` | + +### Example: Upload Failure Correlation + +If logs show: +``` +ERROR: HTTP 403 Forbidden - pre-sign request failed +ERROR: retry_logic: Max retries exhausted for Direct path +``` + +1. Check `uploadstblogs/src/upload_engine.c` for pre-sign logic +2. Check `uploadstblogs/src/retry_logic.c` for retry configuration +3. Verify authentication configuration in `/nvram/DCMresponse.txt` +4. Check certificate paths and OAuth token generation + +--- + +## Step 5: Reproduce Locally + +Create a minimal reproduction scenario: + +### For Configuration Issues +```c +// Test configuration parsing +#include +#include +#include +#include "dcm_parseconf.h" + +void test_parse_bad_config(void) +{ + DCMDHandle handle; + FILE *f; + int ret; + + /* Initialize handle to a known state */ + memset(&handle, 0, sizeof(handle)); + + /* Create test config with issue */ + f = fopen("/tmp/test_dcmresponse.txt", "w"); + if (f == NULL) { + perror("fopen failed"); + return; + } + + if (fprintf(f, "{invalid json}") < 0) { + perror("fprintf failed"); + (void)fclose(f); + return; + } + + if (fclose(f) != 0) { + perror("fclose failed"); + return; + } + + ret = dcmParseConfig(&handle, "/tmp/test_dcmresponse.txt"); + /* Should fail gracefully */ + assert(ret != 0); +} +``` + +### For Upload Issues +```bash +# Test uploadSTBLogs manually +export LOG_PATH=/opt/logs/ +export PERSISTENT_PATH=/opt/ +export DCM_FLAG=1 +export UploadOnReboot=1 + +# Run with debug logging +DEBUG=1 ./uploadstblogs 2>&1 | tee upload_debug.log +``` + +### For RBUS Issues +```bash +# Check RBUS daemon +systemctl status rtrouted + +# Test RBUS method invocation +rbuscli get Device.DeviceInfo.X_RDKCENTRAL-COM_RFC.Feature.DCM.Enable +``` + +--- + +## Step 6: Test Gap Analysis + +Identify untested code paths that could harbor the bug: + +### Check Unit Test Coverage +```bash +# Generate coverage report +./configure --enable-gcov +make clean && make check +gcov *.c +``` + +Look for: +- Error path coverage in suspected functions +- Configuration parsing edge cases +- Network error handling +- Retry logic branches +- Strategy selection conditions + +### Check L2 Test Coverage +Review `test/functional-tests/tests/` for: +- Missing test scenarios matching the bug +- Edge cases not covered +- Error injection tests + +--- + +## Step 7: Propose Fix and Test + +### Fix Template +```c +// BEFORE: Missing error check +int ret = upload_to_s3(archive_path); +// Continue without checking ret + +// AFTER: Proper error handling +int ret = upload_to_s3(archive_path); +if (ret != 0) { + DCM_LOG_ERROR("Upload failed: %d", ret); + // Trigger retry logic or fallback + return handle_upload_error(ret); +} +``` + +### Test Template +```cpp +// Add unit test for the fix +TEST(UploadEngineTest, HandleUploadFailureGracefully) { + // Mock upload failure + EXPECT_CALL(mockCurl, curl_easy_perform(_)) + .WillOnce(Return(CURLE_COULDNT_CONNECT)); + + int ret = upload_to_s3("test.tgz"); + + // Verify error handling + EXPECT_NE(ret, 0); + // Verify cleanup happened + EXPECT_FALSE(file_exists("test.tgz")); +} +``` + +--- + +## Output Format + +Present findings in this structure: + +```markdown +## Triage Summary + +**Issue:** +**Evidence:** +**Root Cause:** +**Impact:** + +## Code Location + +**File:** +**Function:** +**Line:** + +## Reproduction + +[bash or C code to reproduce] + +## Proposed Fix + +[code diff or description] + +## Test Coverage + +**Existing:** [what tests exist] +**Missing:** [tests needed to prevent regression] + +## Next Steps + +1. [immediate action] +2. [follow-up verification] +``` + +--- + +## Example Triage Flow + +**User:** "uploadSTBLogs keeps trying to upload but fails with HTTP 403" + +**Step 1:** Located `uploadstblogs.log.0`, found repeated: +``` +2026-03-24 10:15:32 ERROR: Pre-sign request failed: HTTP 403 Forbidden +2026-03-24 10:15:42 INFO: Retry attempt 2/5 +2026-03-24 10:15:52 ERROR: Pre-sign request failed: HTTP 403 Forbidden +``` + +**Step 2:** Checked `/nvram/DCMresponse.txt` — found OAuth token field empty + +**Step 3:** In `uploadstblogs/src/upload_engine.c`, pre-sign logic doesn't validate +OAuth configuration before attempting request + +**Root Cause:** Missing validation of OAuth token before making pre-sign request + +**Fix:** Add validation in `prepare_upload_request()`: +```c +if (auth_type == AUTH_TYPE_OAUTH && !config->oauth_token) { + DCM_LOG_ERROR("OAuth token not configured"); + return ERR_INVALID_CONFIG; +} +``` + +**Test:** Add `TEST(UploadEngineTest, RejectMissingOAuthToken)`