Skip to content

Commit

Permalink
fix rotating file handler when append mode is used
Browse files Browse the repository at this point in the history
  • Loading branch information
odygrd committed May 15, 2022
1 parent 6b0941b commit ac62be2
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 1 deletion.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Expand Up @@ -29,6 +29,9 @@ From version `v2.0` and onwards only C++17 is supported.

This version is a major refactor.

**Fixes**
- RotatingFileHandler will now correctly rotate the files when append mode is used ([#123](https://github.com/odygrd/quill/issues/123))

**Improvements**

- Reduced and simplified codebase.
Expand Down
50 changes: 49 additions & 1 deletion quill/src/handlers/RotatingFileHandler.cpp
Expand Up @@ -18,7 +18,55 @@ RotatingFileHandler::RotatingFileHandler(std::filesystem::path const& base_filen
_backup_count(backup_count),
_overwrite_oldest_files(overwrite_oldest_files)
{
_file = detail::open_file(_filename, mode);
// if we are starting in w mode, then we also should clean all previous log files of the previous run
if (mode == "w")
{
for (const auto& entry :
std::filesystem::directory_iterator(std::filesystem::current_path() / base_filename.parent_path()))
{
std::size_t found = entry.path().string().find(base_filename.stem().string() + ".");
if (found != std::string::npos)
{
std::filesystem::remove(entry);
}
}
}

This comment has been minimized.

Copy link
@anluoma

anluoma May 19, 2022

Contributor

This comment is for the code starting if (mode == "w")
@odygrd Hmm isn't this quite radical to remove all previous logs when program is restarted? Previously it deleted the current/last file and not all.
I think Wiki example should use the 'a' mode by default?

This comment has been minimized.

Copy link
@odygrd

odygrd May 19, 2022

Author Owner

Previously you could use ‘w’ and end up with a mix of log files from the previous run. Eg the application first time rotated the log 5 times and after restart with ‘w’ it rotates only twice. Usually the intention when using ‘w’ is to replace the old logs. We can add a flag ‘clean’

else if (mode == "a")
{
// Since we are appending, we need to find the current index
for (const auto& entry :
std::filesystem::directory_iterator(std::filesystem::current_path() / base_filename.parent_path()))
{
std::size_t found = entry.path().string().find(base_filename.stem().string() + ".");
if (found != std::string::npos)
{
// stem will be something like `logfile.1`
size_t pos = entry.path().stem().string().find_last_of(".");
if (pos != std::string::npos)
{
std::string index = entry.path().stem().string().substr(pos + 1, entry.path().stem().string().length());

// Attempt to convert the index to a number
try
{
uint32_t index_num = std::stoul(index);
_current_index = (std::max)(_current_index, index_num);
if (_current_index > (_backup_count - 1))
{
// don't increment past _backup_count
_current_index = (_backup_count - 1);
}
}
catch (...)
{
continue;
}
}
}
}
}

_file = detail::open_file(_filename, mode);
_current_size = detail::file_size(_filename);
}

Expand Down

0 comments on commit ac62be2

Please sign in to comment.