Re: [Xen-devel] [PATCH 6/6] xenconsoled: handle --log-backups 0 in logfile_rollover

[Ian Jackson]

Wei Liu writes ("[PATCH 6/6] xenconsoled: handle --log-backups 0 in 
logfile_rollover"):
> We now allow user to configure the number of backups to keep. We need to
> handle when the number is set to 0.
> 
> Check if number of backup is 0. If so, just unlink the file. Move the
> rotation to `else' branch so that we skip it altogether.

I would prefer to review one algorithm correct from the start...

Thanks,
Ian.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH 6/6] xenconsoled: handle --log-backups 0 in logfile_rollover

[Wei Liu]

We now allow user to configure the number of backups to keep. We need to
handle when the number is set to 0.

Check if number of backup is 0. If so, just unlink the file. Move the
rotation to `else' branch so that we skip it altogether.

Signed-off-by: Wei Liu 
---
 tools/console/daemon/logfile.c | 63 +-
 1 file changed, 37 insertions(+), 26 deletions(-)

diff --git a/tools/console/daemon/logfile.c b/tools/console/daemon/logfile.c
index 3b95f84..fd29ba5 100644
--- a/tools/console/daemon/logfile.c
+++ b/tools/console/daemon/logfile.c
@@ -114,38 +114,49 @@ static int logfile_rollover(struct logfile *file)
char *next = NULL;
unsigned i;
 
-   if (asprintf(, "%s.%u", file->basepath,
-log_backups) < 0) {
-   dolog(LOG_ERR, "Failed to asprintf %s.%u",
- file->basepath, log_backups);
-   goto err;
-   }
+   if (log_backups == 0) {
+   if (unlink(file->basepath) < 0 &&
+   errno != ENOENT) {
+   dolog(LOG_ERR, "Failed to unlink %s",
+ file->basepath);
+   goto err;
+   }
+   } else {
+   if (asprintf(, "%s.%u", file->basepath,
+log_backups) < 0) {
+   dolog(LOG_ERR, "Failed to asprintf %s.%u",
+ file->basepath, log_backups);
+   goto err;
+   }
 
-   for (i = log_backups; i > 0; i--) {
-   if (i == 1) {
-   this = strdup(file->basepath);
-   if (!this) {
-   dolog(LOG_ERR, "Failed to strdup %s",
- file->basepath);
-   goto err;
+   for (i = log_backups; i > 0; i--) {
+   if (i == 1) {
+   this = strdup(file->basepath);
+   if (!this) {
+   dolog(LOG_ERR, "Failed to strdup %s",
+ file->basepath);
+   goto err;
+   }
+   } else {
+   if (asprintf(, "%s.%u", file->basepath,
+i-1) < 0) {
+   dolog(LOG_ERR,
+ "Failed to asprintf %s.%u",
+ file->basepath, i-1);
+   goto err;
+   }
}
-   } else {
-   if (asprintf(, "%s.%u", file->basepath, i-1) < 0) {
-   dolog(LOG_ERR, "Failed to asprintf %s.%u",
- file->basepath, i-1);
+
+   if (rename(this, next) < 0 && errno != ENOENT) {
+   dolog(LOG_ERR, "Failed to rename %s to %s",
+ this, next);
goto err;
}
-   }
 
-   if (rename(this, next) < 0 && errno != ENOENT) {
-   dolog(LOG_ERR, "Failed to rename %s to %s",
- this, next);
-   goto err;
+   free(next);
+   next = this;
+   this = NULL;
}
-
-   free(next);
-   next = this;
-   this = NULL;
}
 
new = logfile_entry_new(file->basepath, file->mode);
-- 
2.1.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel