Fix #3379

include/spdlog/sinks/rotating_file_sink-inl.h

@@ -14,7 +14,6 @@
 #include <spdlog/fmt/fmt.h>
 
 #include <cerrno>
-#include <chrono>
 #include <ctime>
 #include <mutex>
 #include <string>
@@ -38,8 +37,8 @@ SPDLOG_INLINE rotating_file_sink<Mutex>::rotating_file_sink(
         throw_spdlog_ex("rotating sink constructor: max_size arg cannot be zero");
     }
 
-    if (max_files > 200000) {
+    if (max_files > MaxFiles) {
-        throw_spdlog_ex("rotating sink constructor: max_files arg cannot exceed 200000");
+        throw_spdlog_ex("rotating sink constructor: max_files arg cannot exceed MaxFiles");
     }
     file_helper_.open(calc_filename(base_filename_, 0));
     current_size_ = file_helper_.size();  // expensive. called only once
@@ -54,11 +53,12 @@ SPDLOG_INLINE rotating_file_sink<Mutex>::rotating_file_sink(
 template <typename Mutex>
 SPDLOG_INLINE filename_t rotating_file_sink<Mutex>::calc_filename(const filename_t &filename,
                                                                   std::size_t index) {
-    if (index == 0u) {
+    if (index == 0U) {
         return filename;
     }
 
-    filename_t basename, ext;
+    filename_t basename;
+    filename_t ext;
     std::tie(basename, ext) = details::file_helper::split_by_extension(filename);
     return fmt_lib::format(SPDLOG_FMT_STRING(SPDLOG_FILENAME_T("{}.{}{}")), basename, index, ext);
 }
@@ -74,6 +74,35 @@ SPDLOG_INLINE void rotating_file_sink<Mutex>::rotate_now() {
     std::lock_guard<Mutex> lock(base_sink<Mutex>::mutex_);
     rotate_();
 }
+template <typename Mutex>
+SPDLOG_INLINE void rotating_file_sink<Mutex>::set_max_size(std::size_t max_size) {
+    std::lock_guard<Mutex> lock(base_sink<Mutex>::mutex_);
+    if (max_size == 0) {
+        throw_spdlog_ex("rotating sink set_max_size: max_size arg cannot be zero");
+    }
+    max_size_ = max_size;
+}
+
+template <typename Mutex>
+SPDLOG_INLINE std::size_t rotating_file_sink<Mutex>::get_max_size() {
+    std::lock_guard<Mutex> lock(base_sink<Mutex>::mutex_);
+    return max_size_;
+}
+
+template <typename Mutex>
+SPDLOG_INLINE void rotating_file_sink<Mutex>::set_max_files(std::size_t max_files) {
+    std::lock_guard<Mutex> lock(base_sink<Mutex>::mutex_);
+    if (max_files > MaxFiles) {
+        throw_spdlog_ex("rotating sink set_max_files: max_files arg cannot exceed 200000");
+    }
+    max_files_ = max_files;
+}
+
+template <typename Mutex>
+std::size_t rotating_file_sink<Mutex>::get_max_files(){
+    std::lock_guard<Mutex> lock(base_sink<Mutex>::mutex_);
+    return max_files_;
+}
 
 template <typename Mutex>
 SPDLOG_INLINE void rotating_file_sink<Mutex>::sink_it_(const details::log_msg &msg) {

include/spdlog/sinks/rotating_file_sink.h

@@ -8,7 +8,6 @@
 #include <spdlog/details/synchronous_factory.h>
 #include <spdlog/sinks/base_sink.h>
 
-#include <chrono>
 #include <mutex>
 #include <string>
 
@@ -21,6 +20,7 @@ namespace sinks {
 template <typename Mutex>
 class rotating_file_sink final : public base_sink<Mutex> {
 public:
+    static constexpr size_t MaxFiles = 200000;
     rotating_file_sink(filename_t base_filename,
                        std::size_t max_size,
                        std::size_t max_files,
@@ -29,6 +29,10 @@ public:
     static filename_t calc_filename(const filename_t &filename, std::size_t index);
     filename_t filename();
     void rotate_now();
+    void set_max_size(std::size_t max_size);
+    std::size_t get_max_size();
+    void set_max_files(std::size_t max_files);
+    std::size_t get_max_files();
 
 protected:
     void sink_it_(const details::log_msg &msg) override;
@@ -42,7 +46,7 @@ private:
     // log.3.txt -> delete
     void rotate_();
 
-    // delete the target if exists, and rename the src file  to target
+    // delete the target if exists, and rename the src file to target
     // return true on success, false otherwise.
     bool rename_file_(const filename_t &src_filename, const filename_t &target_filename);
 
@@ -61,9 +65,8 @@ using rotating_file_sink_st = rotating_file_sink<details::null_mutex>;
 //
 // factory functions
 //
-
 template <typename Factory = spdlog::synchronous_factory>
-inline std::shared_ptr<logger> rotating_logger_mt(const std::string &logger_name,
+std::shared_ptr<logger> rotating_logger_mt(const std::string &logger_name,
                                                   const filename_t &filename,
                                                   size_t max_file_size,
                                                   size_t max_files,
@@ -74,7 +77,7 @@ inline std::shared_ptr<logger> rotating_logger_mt(const std::string &logger_name
 }
 
 template <typename Factory = spdlog::synchronous_factory>
-inline std::shared_ptr<logger> rotating_logger_st(const std::string &logger_name,
+std::shared_ptr<logger> rotating_logger_st(const std::string &logger_name,
                                                   const filename_t &filename,
                                                   size_t max_file_size,
                                                   size_t max_files,

tests/test_file_logging.cpp

@@ -56,7 +56,7 @@ TEST_CASE("simple_file_logger", "[truncate]") {
     logger->info("Test message {}", 2.71);
     logger->flush();
     REQUIRE(count_lines(SIMPLE_LOG) == 2);
-    
+
     sink->truncate();
     REQUIRE(count_lines(SIMPLE_LOG) == 0);
 
@@ -94,14 +94,11 @@ TEST_CASE("rotating_file_logger2", "[rotating_logger]") {
         // next logger can rename the first output file.
         spdlog::drop(logger->name());
     }
-
     auto logger = spdlog::rotating_logger_mt("logger", basename, max_size, 2, true);
     for (int i = 0; i < 10; ++i) {
         logger->info("Test message {}", i);
     }
-
     logger->flush();
-
     require_message_count(ROTATING_LOG, 10);
 
     for (int i = 0; i < 1000; i++) {
@@ -141,3 +138,50 @@ TEST_CASE("rotating_file_logger4", "[rotating_logger]") {
     REQUIRE(get_filesize(ROTATING_LOG) > 0);
     REQUIRE(get_filesize(ROTATING_LOG ".1") > 0);
 }
+
+// test changing the max size of the rotating file sink
+TEST_CASE("rotating_file_logger5", "[rotating_logger]") {
+    prepare_logdir();
+    size_t max_size = 5 * 1024;
+    size_t max_files = 2;
+    spdlog::filename_t basename = SPDLOG_FILENAME_T(ROTATING_LOG);
+    auto sink =
+        std::make_shared<spdlog::sinks::rotating_file_sink_st>(basename, max_size, max_files);
+    auto logger = std::make_shared<spdlog::logger>("rotating_sink_logger", sink);
+    logger->set_pattern("%v");
+
+    REQUIRE(sink->get_max_size() == max_size);
+    REQUIRE(sink->get_max_files() == max_files);
+    max_size = 7 * 1024;
+    max_files = 3;
+
+    sink->set_max_size(max_size);
+    sink->set_max_files(max_files);
+    REQUIRE(sink->get_max_size() == max_size);
+    REQUIRE(sink->get_max_files() == max_files);
+
+    const auto message = std::string(200, 'x');
+    assert(message.size() < max_size);
+    const auto n_messages = max_files * max_size / message.size();
+    for (int i = 0; i < n_messages; ++i) {
+        logger->info(message, i);
+    }
+    logger.reset(); // force flush and close the file
+
+    // validate that the files were rotated correctly with the new max size and max files
+    for (int i = 0; i <= max_files; i++) {
+        // calc filenames
+        // e.g. rotating_log, rotating_log.0 rotating_log.1, rotating_log.2, etc.
+        std::ostringstream oss;
+        oss << ROTATING_LOG;
+        if (i > 0) {
+            oss << '.' << i;
+        }
+        const auto filename = oss.str();
+        const auto filesize = get_filesize(filename);
+        REQUIRE(filesize <= max_size);
+        if (i > 0) {
+            REQUIRE(filesize >= max_size - message.size() - 2);
+        }
+    }
+}

tests/utils.cpp

@@ -50,10 +50,9 @@ void require_message_count(const std::string &filename, const std::size_t messag
 std::size_t get_filesize(const std::string &filename) {
     std::ifstream ifs(filename, std::ifstream::ate | std::ifstream::binary);
     if (!ifs) {
-        throw std::runtime_error("Failed open file ");
+        throw std::runtime_error("Failed open file " + filename);
     }
-
+    return ifs.tellg();
-    return static_cast<std::size_t>(ifs.tellg());
 }
 
 // source: https://stackoverflow.com/a/2072890/192001