-
Notifications
You must be signed in to change notification settings - Fork 322
logstorage: Use correct fd for fsync on gzip log files #761
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
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: lum3hc <Minh.LuuQuang@vn.bosch.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes a file synchronization issue in the DLT offline log storage system. The change corrects the file descriptor used for fsync operations when dealing with gzip-compressed log files.
- Replaces
fsync(fileno(config->log))withfsync(config->fd)for proper file synchronization - Adds validation to ensure the file descriptor is valid before attempting fsync
- Improves error handling with appropriate error messages for invalid file descriptors
Comments suppressed due to low confidence (1)
src/offlinelogstorage/dlt_offline_logstorage_behavior.c:1125
- [nitpick] The error message could be more descriptive by including the actual file descriptor value to aid in debugging. Consider: "invalid file descriptor (%d) for log file sync"
dlt_vlog(LOG_ERR, "%s: invalid file descriptor for log file sync\n", __func__);
|
Kindly review @lti9hc |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
c4da3ef to
890c6ba
Compare
|
Correct commit msg and commit title -> no gzip related |
|
As the project was referenced automatically anyway but github, I wanted to share the dlt rust wrapper we just open sourced. Rust is more and more used in the automative world, so I was wondering if you're interested in linking the dlt crates I posted above somewhere in docs or readme.md? |
|
Hello please also check the content of #788 and integrate it, there are some improvements too |
No description provided.