Skip to content

Feature/shell log #109

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Abhinavpv28 wants to merge 6 commits into
base: develop
Choose a base branch
from
Open

Feature/shell log #109

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ef3e491
Update backup_logs.c
Abhinavpv28 Apr 1, 2026
d634268
Update backup_engine.c
Abhinavpv28 Apr 1, 2026
6ee21a3
Update backup_logs.h
Abhinavpv28 Apr 1, 2026
e0b542a
Update backup_logs/src/backup_logs.c
Abhinavpv28 Apr 1, 2026
9bb9f60
Update backup_logs/src/backup_logs.c
Abhinavpv28 Apr 1, 2026
1d910c2
Update backup_logs/src/backup_logs.c
Abhinavpv28 Apr 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions backup_logs/include/backup_logs.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
/*
* If not stated otherwise in this file or this component's LICENSE file the
* following copyright and licenses apply:
* If not stated otherwise in this file or this component's LICENSE
* file the following copyright and licenses apply:
*
* Copyright 2026 RDK Management
* Copyright 2024 Comcast Cable Communications Management, LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -15,6 +15,8 @@
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
* SPDX-License-Identifier: Apache-2.0
*/

#ifndef BACKUP_LOGS_H
Expand Down
34 changes: 27 additions & 7 deletions backup_logs/src/backup_engine.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,26 @@

/* RDK Logging component name for Backup Logs */

/* Backup-specific symlink-aware file existence check
* This function checks if a file or symlink exists, regardless of whether the symlink target exists
* Uses lstat() instead of stat() to examine the symlink itself, not its target
*/
static int backup_file_exists_check(const char *file_name) {
if (!file_name) {
return -1; // Invalid parameter
}

struct stat sfile;
memset(&sfile, 0, sizeof(sfile));

/* Use lstat() to check the file/symlink itself, not the target */
if (lstat(file_name, &sfile) != 0) {
return -1; // File/symlink doesn't exist
}

return 0; // File or symlink exists
Comment on lines +47 to +64

Copilot AI Apr 1, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switching from filePresentCheck() to a static helper that calls lstat() will bypass the existing unit-test mocks: backup_logs/unittest/backup_engine_gtest.cpp wraps filePresentCheck/stat but does not wrap lstat, and many tests assert filePresentCheck_called. As-is, the tests will start hitting the real filesystem or fail assertions. Consider providing a non-static wrapper that tests can __wrap_..., adding an __wrap_lstat, or updating the tests accordingly.

Suggested change
/* Backup-specific symlink-aware file existence check
* This function checks if a file or symlink exists, regardless of whether the symlink target exists
* Uses lstat() instead of stat() to examine the symlink itself, not its target
*/
static int backup_file_exists_check(const char *file_name) {
if (!file_name) {
return -1; // Invalid parameter
}
struct stat sfile;
memset(&sfile, 0, sizeof(sfile));
/* Use lstat() to check the file/symlink itself, not the target */
if (lstat(file_name, &sfile) != 0) {
return -1; // File/symlink doesn't exist
}
return 0; // File or symlink exists
/* Backup-specific file existence check
* This function wraps filePresentCheck() so that existing unit-test mocks
* and instrumentation (e.g. filePresentCheck_called) continue to work.
*/
static int backup_file_exists_check(const char *file_name) {
if (!file_name) {
return -1; /* Invalid parameter */
}
/* Delegate to the existing, test-mocked file existence helper. */
return filePresentCheck(file_name);

Copilot uses AI. Check for mistakes.
Comment on lines +47 to +64

Copilot AI Apr 1, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Symlink-aware existence checking logic is now duplicated in this file (backup_file_exists_check) and in backup_logs.c (backup_filePresentCheck_symlink_aware), with different names and slightly different comments. To avoid drift, consider centralizing this check in a single utility function (e.g., in a shared module/header) and reusing it from both places.

Suggested change
/* Backup-specific symlink-aware file existence check
* This function checks if a file or symlink exists, regardless of whether the symlink target exists
* Uses lstat() instead of stat() to examine the symlink itself, not its target
*/
static int backup_file_exists_check(const char *file_name) {
if (!file_name) {
return -1; // Invalid parameter
}
struct stat sfile;
memset(&sfile, 0, sizeof(sfile));
/* Use lstat() to check the file/symlink itself, not the target */
if (lstat(file_name, &sfile) != 0) {
return -1; // File/symlink doesn't exist
}
return 0; // File or symlink exists
/* Shared symlink-aware file existence check implemented in backup_logs.c
* This function checks if a file or symlink exists, regardless of whether the
* symlink target exists. It uses lstat() internally to examine the symlink
* itself, not its target.
*/
extern int backup_filePresentCheck_symlink_aware(const char *file_name);
/* Backup-specific wrapper for the shared symlink-aware file existence check.
* Keeping this function preserves the local interface while centralizing the
* actual existence-checking logic in backup_filePresentCheck_symlink_aware().
*/
static int backup_file_exists_check(const char *file_name) {
return backup_filePresentCheck_symlink_aware(file_name);

Copilot uses AI. Check for mistakes.
}


/* Helper function to move log files matching patterns */
int move_log_files_by_pattern(const char* source_dir, const char* dest_dir) {
Expand All @@ -70,8 +90,8 @@ int move_log_files_by_pattern(const char* source_dir, const char* dest_dir) {
continue;
}

/* Check if it's a regular file */
if (filePresentCheck(source_file) != 0) {
/* Check if it's a file or symlink (using symlink-aware check) */
if (backup_file_exists_check(source_file) != 0) {
continue;
}

Expand Down Expand Up @@ -132,7 +152,7 @@ int backup_execute_hdd_enabled_strategy(const backup_config_t* config) {
strcat(syslog_path, "/");
strcat(syslog_path, sysLog);

if (filePresentCheck(syslog_path) != 0) {
if (backup_file_exists_check(syslog_path) != 0) {
RDK_LOG(RDK_LOG_INFO, LOG_BACKUP_LOGS, "First time backup - moving logs to %s\n", config->prev_log_path);
/* First time - move logs directly to PREV_LOG_PATH */
move_log_files_by_pattern(config->log_path, config->prev_log_path);
Expand Down Expand Up @@ -279,17 +299,17 @@ int backup_execute_hdd_disabled_strategy(const backup_config_t* config) {
strcpy(prev_log_path_slash, config->prev_log_path); strcat(prev_log_path_slash, "/");

/* HDD disabled backup rotation logic */
if (filePresentCheck(syslog_path) != 0) {
if (backup_file_exists_check(syslog_path) != 0) {
/* First time - move all logs directly */
RDK_LOG(RDK_LOG_INFO, LOG_BACKUP_LOGS, "First time HDD-disabled backup - moving all logs\n");
backup_and_recover_logs(log_path_slash, prev_log_path_slash, BACKUP_OP_MOVE, "", "");
} else if (filePresentCheck(bak1_path) != 0) {
} else if (backup_file_exists_check(bak1_path) != 0) {
RDK_LOG(RDK_LOG_INFO, LOG_BACKUP_LOGS, "Moving logs to bak1_ prefix\n");
backup_and_recover_logs(log_path_slash, prev_log_path_slash, BACKUP_OP_MOVE, "", "bak1_");
} else if (filePresentCheck(bak2_path) != 0) {
} else if (backup_file_exists_check(bak2_path) != 0) {
RDK_LOG(RDK_LOG_INFO, LOG_BACKUP_LOGS, "Moving logs to bak2_ prefix\n");
backup_and_recover_logs(log_path_slash, prev_log_path_slash, BACKUP_OP_MOVE, "", "bak2_");
} else if (filePresentCheck(bak3_path) != 0) {
} else if (backup_file_exists_check(bak3_path) != 0) {
RDK_LOG(RDK_LOG_INFO, LOG_BACKUP_LOGS, "Moving logs to bak3_ prefix\n");
backup_and_recover_logs(log_path_slash, prev_log_path_slash, BACKUP_OP_MOVE, "", "bak3_");
} else {
Expand Down
19 changes: 15 additions & 4 deletions backup_logs/src/backup_logs.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
/*
* If not stated otherwise in this file or this component's LICENSE file the
* following copyright and licenses apply:
* If not stated otherwise in this file or this component's LICENSE
* file the following copyright and licenses apply:
*
* Copyright 2026 RDK Management
* Copyright 2024 Comcast Cable Communications Management, LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -15,13 +15,17 @@
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
* SPDX-License-Identifier: Apache-2.0
*/

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <getopt.h>
#include <time.h>
#include <sys/stat.h>



Comment on lines +28 to 31

Copilot AI Apr 1, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

<sys/time.h> and <sys/stat.h> appear unused in this file after the changes (the timing code uses clock_gettime/timespec from <time.h>). Consider removing unused headers to keep dependencies minimal and avoid portability issues on constrained toolchains.

Suggested change
#include <sys/stat.h>

Copilot uses AI. Check for mistakes.
Expand Down Expand Up @@ -264,6 +268,10 @@ int backup_logs_cleanup(backup_config_t *config) {

/* Main entry point */
int backup_logs_main(int argc, char *argv[]) {
/* Start timing the program execution */
struct timespec program_start, program_end;
clock_gettime(CLOCK_MONOTONIC, &program_start);

Comment on lines +271 to +274

Copilot AI Apr 1, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clock_gettime(CLOCK_MONOTONIC, ...) return values are not checked. If either call fails (e.g., unsupported clock id on some platforms), program_start/program_end may contain garbage and the runtime log becomes incorrect. Please handle failures (e.g., skip timing/log an error) or guard the calculation on successful calls.

Copilot uses AI. Check for mistakes.
RDK_LOG(RDK_LOG_DEBUG, LOG_BACKUP_LOGS, "Starting backup_logs main function with %d arguments\n", argc);

/* Suppress unused parameter warnings */
Expand Down Expand Up @@ -301,7 +309,10 @@ int backup_logs_main(int argc, char *argv[]) {
return EXIT_FAILURE;
}

RDK_LOG(RDK_LOG_INFO, LOG_BACKUP_LOGS, "Backup process completed successfully\n");
/* Calculate and log total execution time */
clock_gettime(CLOCK_MONOTONIC, &program_end);
double total_time = (program_end.tv_sec - program_start.tv_sec) + (program_end.tv_nsec - program_start.tv_nsec) / 1000000000.0;
RDK_LOG(RDK_LOG_INFO, LOG_BACKUP_LOGS, "Backup process completed successfully - Total runtime: %.3f seconds\n", total_time);
return EXIT_SUCCESS;
Comment on lines +313 to 316

Copilot AI Apr 1, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The runtime measurement uses double arithmetic for seconds, which can increase code size and CPU cost on embedded targets (especially without an FPU). Consider logging an integer duration (e.g., milliseconds as int64_t) to avoid floating-point and keep the binary lightweight.

Copilot uses AI. Check for mistakes.
}
#ifndef GTEST_ENABLE
Expand Down
Loading