You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2021/06/04 17:33:50 UTC
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #3848: Syslog file rotations number is configurable.
xiaoxiang781216 commented on a change in pull request #3848:
URL: https://github.com/apache/incubator-nuttx/pull/3848#discussion_r645739872
##########
File path: drivers/syslog/syslog_filechannel.c
##########
@@ -85,28 +89,44 @@ static void log_rotate(FAR const char *log_file)
return;
}
- /* Construct the backup file name. */
+ /* Rotated file names. */
- backup_file = malloc(strlen(log_file) + 3);
- if (backup_file == NULL)
+ name_size = strlen(log_file) + 8;
+ rotate_to = malloc(name_size);
+ rotate_from = malloc(name_size);
+ if ((rotate_to == NULL) || (rotate_from == NULL))
{
- return;
+ goto end;
}
- sprintf(backup_file, "%s.0", log_file);
+ /* Rotate the logs. */
+
+ for (i = (CONFIG_SYSLOG_FILE_ROTATIONS - 1); i > 0; i--)
+ {
+ snprintf(rotate_to, name_size, "%s.%d", log_file, i);
+ snprintf(rotate_from, name_size, "%s.%d", log_file, i - 1);
+
+ if (access(rotate_to, F_OK) == 0)
+ {
+ remove(rotate_to);
+ }
+
+ rename(rotate_from, rotate_to);
+ }
- /* Delete any old backup files. */
+ snprintf(rotate_to, name_size, "%s.0", log_file);
- if (access(backup_file, F_OK) == 0)
+ if (access(rotate_to, F_OK) == 0)
{
- remove(backup_file);
+ remove(rotate_to);
}
- /* Rotate the log. */
+ rename(log_file, rotate_to);
- rename(log_file, backup_file);
+end:
- free(backup_file);
+ free(rotate_to);
+ free(rotate_from);
Review comment:
kmm_free
##########
File path: drivers/syslog/Kconfig
##########
@@ -286,6 +286,13 @@ config SYSLOG_FILE_ROTATE
This option is useful to ensure that log files do not get
huge after prolonged periods of system operation.
+config SYSLOG_FILE_ROTATIONS
+ int "Log rotations"
+ default 1
Review comment:
we can remove SYSLOG_FILE_ROTATE and set SYSLOG_FILE_ROTATIONS = 0 to disable rotation.
##########
File path: drivers/syslog/syslog_filechannel.c
##########
@@ -85,28 +89,44 @@ static void log_rotate(FAR const char *log_file)
return;
}
- /* Construct the backup file name. */
+ /* Rotated file names. */
- backup_file = malloc(strlen(log_file) + 3);
- if (backup_file == NULL)
+ name_size = strlen(log_file) + 8;
+ rotate_to = malloc(name_size);
+ rotate_from = malloc(name_size);
Review comment:
all malloc -> kmm_malloc
##########
File path: drivers/syslog/syslog_filechannel.c
##########
@@ -85,28 +89,44 @@ static void log_rotate(FAR const char *log_file)
return;
}
- /* Construct the backup file name. */
+ /* Rotated file names. */
- backup_file = malloc(strlen(log_file) + 3);
- if (backup_file == NULL)
+ name_size = strlen(log_file) + 8;
+ rotate_to = malloc(name_size);
+ rotate_from = malloc(name_size);
+ if ((rotate_to == NULL) || (rotate_from == NULL))
{
- return;
+ goto end;
}
- sprintf(backup_file, "%s.0", log_file);
+ /* Rotate the logs. */
+
+ for (i = (CONFIG_SYSLOG_FILE_ROTATIONS - 1); i > 0; i--)
+ {
+ snprintf(rotate_to, name_size, "%s.%d", log_file, i);
+ snprintf(rotate_from, name_size, "%s.%d", log_file, i - 1);
+
+ if (access(rotate_to, F_OK) == 0)
+ {
+ remove(rotate_to);
+ }
+
+ rename(rotate_from, rotate_to);
+ }
- /* Delete any old backup files. */
+ snprintf(rotate_to, name_size, "%s.0", log_file);
- if (access(backup_file, F_OK) == 0)
+ if (access(rotate_to, F_OK) == 0)
{
- remove(backup_file);
+ remove(rotate_to);
}
- /* Rotate the log. */
+ rename(log_file, rotate_to);
Review comment:
line 116-124 can be simplify to:
rename(log_file, rotate_from);
##########
File path: drivers/syslog/syslog_filechannel.c
##########
@@ -85,28 +89,44 @@ static void log_rotate(FAR const char *log_file)
return;
}
- /* Construct the backup file name. */
+ /* Rotated file names. */
- backup_file = malloc(strlen(log_file) + 3);
- if (backup_file == NULL)
+ name_size = strlen(log_file) + 8;
+ rotate_to = malloc(name_size);
+ rotate_from = malloc(name_size);
+ if ((rotate_to == NULL) || (rotate_from == NULL))
{
- return;
+ goto end;
}
- sprintf(backup_file, "%s.0", log_file);
+ /* Rotate the logs. */
+
+ for (i = (CONFIG_SYSLOG_FILE_ROTATIONS - 1); i > 0; i--)
+ {
+ snprintf(rotate_to, name_size, "%s.%d", log_file, i);
+ snprintf(rotate_from, name_size, "%s.%d", log_file, i - 1);
+
+ if (access(rotate_to, F_OK) == 0)
+ {
+ remove(rotate_to);
+ }
+
+ rename(rotate_from, rotate_to);
Review comment:
should we check:
if (access(rotate_from, F_OK) == 0)
{
rename(rotate_from, rotate_to);
}
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org