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.

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

Fix file deletion when rotating and some bugfixes #718

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

Conversation

andreasbank
Copy link
Contributor

  • Fix file deletion when rotating
  • Correct mutexattr init
  • Small code cleanup and improvements
  • Properly terminate log messages

If there are more files than the configuration allows, delete them all,
not just the last one
strncat() already searches for the null-term in src, making strlen()
completely useless.
Without the newline character multiple logs get concatenated
into a single log.
@andreasbank andreasbank changed the title Some bugfixes and improvements Fix file deletion when rotating and some bugfixes Dec 9, 2024
}
if (fsync(config->fd) != 0) {
if (errno != ENOSYS) {
dlt_vlog(LOG_ERR, "%s: failed to sync log file\n", __func__);
Copy link
Collaborator

Choose a reason for hiding this comment

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

could you explain why it is only for non gzip?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is not only for gzip it is still for both cases, since I removed the gzip check.

Since fileno(config->gzlog) is config->fd and fileno(config->log) is also config->fd, then we can replace them both with a with a single fsync() call, regardless if config->gzip_compression is set or not.
The executed functions are exactly the same as before but the code looks a lot simpler.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Check the huge refactor: #732
Thanks

Copy link
Collaborator

@minminlittleshrimp minminlittleshrimp Feb 20, 2025

Choose a reason for hiding this comment

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

It is not only for gzip it is still for both cases, since I removed the gzip check.

Since fileno(config->gzlog) is config->fd and fileno(config->log) is also config->fd, then we can replace them both with a with a single fsync() call, regardless if config->gzip_compression is set or not. The executed functions are exactly the same as before but the code looks a lot simpler.

Not correct, gzlog is *gzFile, it could be handled like this:

if (config->gzlog && gzdirect(*config->gzlog)) {
        int fd = fileno((FILE *)*config->gzlog);
        if (fd != -1 && fsync(fd) != 0) {
            if (errno != ENOSYS) {
                dlt_vlog(LOG_ERR, "%s: failed to sync gzip log file\n", __func__);

@minminlittleshrimp
Copy link
Collaborator

Close by mistake, please check #732 for upcoming vers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants