Skip to content
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

logrotate: update the logdir path whenever it changes #501

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

logrotate: update the logdir path whenever it changes #501

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
76 changes: 75 additions & 1 deletion libtcmu_log.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
*/

#define _GNU_SOURCE
#include <stdio.h>
#include <stdint.h>
#include <pthread.h>
#include <unistd.h>
Expand Down Expand Up @@ -524,6 +525,78 @@ static bool tcmu_log_dir_check(const char *path)
return true;
}

#define TCMU_LOGROTATE_PATH "/etc/logrotate.d/tcmu-runner"
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be part of a header file ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Currently, only used here. IMO, both will be okay.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh yes, this is perfectly okay.
My intention was, considering declaration of potentially common macros (such as this) in headers will reduce rework.


static int tcmu_logrotate_conf_set(const char *logdir)
{
char *buf = NULL, *line = NULL, *p;
int ret, m, len;
size_t n;
FILE *fp;

fp = fopen(TCMU_LOGROTATE_PATH, "r+");
if (fp == NULL) {
ret = -errno;
tcmu_err("Failed to open file '%s', %m\n", TCMU_LOGROTATE_PATH);
return ret;
}

ret = fseek(fp, 0L, SEEK_END);
if (ret == -1) {
ret = -errno;
Copy link
Contributor

Choose a reason for hiding this comment

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

this should be moved before line 545

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah. Will fix it

tcmu_err("Failed to seek file '%s', %m\n", TCMU_LOGROTATE_PATH);
goto error;
}

len = ftell(fp);
if (len == -1) {
ret = -errno;
Copy link
Contributor

Choose a reason for hiding this comment

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

same here

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Will fix it.

tcmu_err("Failed to get the length of file '%s', %m\n", TCMU_LOGROTATE_PATH);
goto error;
}

/* to make sure we have enough size */
len += strlen(logdir) + 1;
buf = calloc(len, sizeof(char));
if (!buf) {
ret = -ENOMEM;
goto error;
}

p = buf;
fseek(fp, 0L, SEEK_SET);
while ((m = getline(&line, &n, fp)) != -1) {
if (strstr(line, "tcmu-runner*.log") && strchr(line, '{'))
m = sprintf(p, "%s/tcmu-runner*.log {\n", logdir);
else
m = sprintf(p, "%s", line);
if (m < 0) {
ret = m;
goto error;
}

p += m;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

did you miss to free(line) in the loop ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No needed to free it every time:

If *lineptr is NULL, then getline() will allocate a buffer for storing the line, which should be freed by the user program.  (In this case, the value  in  *n  is
       ignored.)
Alternatively,  before  calling getline(), *lineptr can contain a pointer to a malloc(3)-allocated buffer *n bytes in size.  If the buffer is not large enough to
       hold the line, getline() resizes it with realloc(3), updating *lineptr and *n as necessary.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I read the manpage now. Thanks for adding the clarity.

*p = '\0';
len = p - buf;

fseek(fp, 0L, SEEK_SET);
truncate(TCMU_LOGROTATE_PATH, 0L);
ret = fwrite(buf, 1, len, fp);
if (ret != len) {
tcmu_err("Failed to update '%s', %m\n", TCMU_LOGROTATE_PATH);
goto error;
}

ret = 0;
error:
if (fp)
fclose(fp);
free(buf);
free(line);
return ret;
}

static int tcmu_log_dir_set(const char *log_dir)
{
char *new_dir;
Expand All @@ -536,7 +609,8 @@ static int tcmu_log_dir_set(const char *log_dir)

tcmu_log_dir_free();
tcmu_log_dir = new_dir;
return 0;

return tcmu_logrotate_conf_set(tcmu_log_dir);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you need to put this call before we free the old and set the new tcmu_log_dir in case the call to tcmu_logrotate_conf_set fails. We then do not have to worry about unraveling what was done here.

}

static int tcmu_mkdir(const char *path)
Expand Down