13 files changed, 228 insertions(+), 68 deletions(-)

M board/rt1051/CMakeLists.txt
M board/rt1051/crashdump/crashdumpwriter_vfs.cpp
M board/rt1051/crashdump/crashdumpwriter_vfs.hpp
M module-utils/CMakeLists.txt
M module-utils/log/CMakeLists.txt
M module-utils/log/Logger.cpp
M module-utils/log/Logger.hpp
M module-utils/log/tests/CMakeLists.txt
M module-utils/log/tests/test_logDumps.cpp
A module-utils/rotator/CMakeLists.txt
A module-utils/rotator/include/rotator/Rotator.hpp
A module-utils/rotator/tests/CMakeLists.txt
A module-utils/rotator/tests/test_Rotator.cpp

@@ 30,6 30,8 @@ target_include_directories(board
 )
 
 target_link_libraries(board
+    PRIVATE
+        utils-rotator
     PUBLIC
         fsl
         module-vfs

@@ 8,21 8,24 @@
 #include "purefs/vfs_subsystem.hpp"
 #include <purefs/filesystem_paths.hpp>
 
+#include <filesystem>
+
 namespace crashdump
 {
+    constexpr inline auto crashDumpFileName = "crashdump.hex";
+
     void CrashDumpWriterVFS::openDump()
     {
-        std::array<char, 64> crashDumpFileName{0};
-        formatCrashDumpFileName(crashDumpFileName);
+        vfs                          = purefs::subsystem::vfs_core();
+        const auto crashDumpFilePath = purefs::dir::getCrashDumpsPath() / crashDumpFileName;
 
-        const auto crashDumpFilePath = purefs::dir::getCrashDumpsPath() / crashDumpFileName.data();
-
-        vfs    = purefs::subsystem::vfs_core();
+        if (!rotator.rotateFile(crashDumpFilePath)) {
+            LOG_FATAL("Failed to rotate crash dumps.");
+        }
         dumpFd = vfs->open(crashDumpFilePath.c_str(), O_WRONLY | O_CREAT | O_TRUNC, 0666);
 
         if (dumpFd < 0) {
-            LOG_FATAL("Failed to open crash dump file [%s]. Won't be able to save crash info.",
-                      crashDumpFilePath.c_str());
+            LOG_FATAL("Failed to open crash dump file. Won't be able to save crash info.");
         }
     }
 

@@ 4,6 4,7 @@
 #pragma once
 
 #include "crashdumpwriter.hpp"
+#include <rotator/Rotator.hpp>
 
 #include <array>
 #include <ctime>


@@ 16,11 17,12 @@ namespace purefs::fs
 
 namespace crashdump
 {
-    constexpr inline auto CrashDumpFileNameFormat = "crashdump-%FT%TZ.hex";
-
+    constexpr inline auto maxRotationFilesCount = 5;
     class CrashDumpWriterVFS : public CrashDumpWriter
     {
       public:
+        CrashDumpWriterVFS() : rotator{".hex"}
+        {}
         void openDump() override;
         void saveDump() override;
 


@@ 29,13 31,7 @@ namespace crashdump
         void writeWords(const std::uint32_t *buff, std::size_t size) override;
 
       private:
-        template <std::size_t N> void formatCrashDumpFileName(std::array<char, N> &name)
-        {
-            std::time_t now;
-            std::time(&now);
-            std::strftime(name.data(), name.size(), CrashDumpFileNameFormat, std::localtime(&now));
-        }
-
+        utils::Rotator<maxRotationFilesCount> rotator;
         int dumpFd{-1};
 
         std::shared_ptr<purefs::fs::filesystem> vfs;

@@ 11,6 11,7 @@ add_subdirectory(locale)
 add_subdirectory(log)
 add_subdirectory(math)
 add_subdirectory(phonenumber)
+add_subdirectory(rotator)
 add_subdirectory(rrule)
 add_subdirectory(time)
 add_subdirectory(unicode)

@@ 17,10 17,10 @@ target_link_libraries(log
     PRIVATE
         utility
         purefs-paths
-
     PUBLIC
         module-os
         log-api
+        utils-rotator    
 )
 
 if (${ENABLE_TESTS})

@@ 12,21 12,6 @@
 
 namespace Log
 {
-    namespace
-    {
-        std::string getRotatedLogFileExtension(int count)
-        {
-            return ".log." + utils::to_string(count);
-        }
-
-        std::filesystem::path getRotatedFilePath(const std::filesystem::path &source, int rotationCount)
-        {
-            auto path = source;
-            path.replace_extension(getRotatedLogFileExtension(rotationCount));
-            return path;
-        }
-    } // namespace
-
     std::map<std::string, logger_level> Logger::filtered = {{"ApplicationManager", logger_level::LOGINFO},
                                                             {"CellularMux", logger_level::LOGINFO},
                                                             {"ServiceCellular", logger_level::LOGINFO},


@@ 47,7 32,7 @@ namespace Log
         return stream;
     }
 
-    Logger::Logger() : circularBuffer(circularBufferSize)
+    Logger::Logger() : circularBuffer{circularBufferSize}, rotator{".log"}
     {}
 
     void Logger::enableColors(bool enable)


@@ 81,11 66,10 @@ namespace Log
         return logs;
     }
 
-    void Logger::init(Application app, size_t fileSize, int filesCount)
+    void Logger::init(Application app, size_t fileSize)
     {
         application      = std::move(app);
         maxFileSize      = fileSize;
-        maxRotationIndex = filesCount - 1;
 #if LOG_USE_COLOR == 1
         enableColors(true);
 #else


@@ 158,7 142,7 @@ namespace Log
             LOG_DEBUG("Max log file size exceeded. Rotating log files...");
             {
                 LockGuard lock(logFileMutex);
-                rotateLogFile(logPath);
+                rotator.rotateFile(logPath);
             }
             firstDump = true;
         }


@@ 187,29 171,6 @@ namespace Log
         return status;
     }
 
-    void Logger::rotateLogFile(const std::filesystem::path &logPath)
-    {
-        for (int i = currentRotationIndex; i > 0; --i) {
-            std::filesystem::path src = getRotatedFilePath(logPath, i);
-            if (i == maxRotationIndex) {
-                std::filesystem::remove(src);
-                continue;
-            }
-            std::filesystem::path dest = getRotatedFilePath(logPath, i + 1);
-            std::filesystem::rename(src, dest);
-        }
-        auto rotatedLogPath = getRotatedFilePath(logPath, 1);
-        std::filesystem::rename(logPath, rotatedLogPath);
-        onLogRotationFinished();
-    }
-
-    void Logger::onLogRotationFinished() noexcept
-    {
-        if (currentRotationIndex < maxRotationIndex) {
-            ++currentRotationIndex;
-        }
-    }
-
     void Logger::addFileHeader(std::ofstream &file) const
     {
         file << application;

@@ 7,6 7,7 @@
 #include <log/log.hpp>
 #include "LoggerBuffer.hpp"
 #include "log_colors.hpp"
+#include <rotator/Rotator.hpp>
 #include <map>
 #include <mutex.hpp>
 #include <string>


@@ 39,7 40,7 @@ namespace Log
             return logger;
         }
         auto getLogs() -> std::string;
-        void init(Application app, size_t fileSize = MAX_LOG_FILE_SIZE, int filesCount = MAX_LOG_FILES_COUNT);
+        void init(Application app, size_t fileSize = MAX_LOG_FILE_SIZE);
         auto log(Device device, const char *fmt, va_list args) -> int;
         auto log(logger_level level, const char *file, int line, const char *function, const char *fmt, va_list args)
             -> int;


@@ 73,8 74,6 @@ namespace Log
         }
 
         void addFileHeader(std::ofstream &file) const;
-        void rotateLogFile(const std::filesystem::path &logPath);
-        void onLogRotationFinished() noexcept;
 
         cpp_freertos::MutexStandard mutex;
         cpp_freertos::MutexStandard logFileMutex;


@@ 83,11 82,10 @@ namespace Log
         char loggerBuffer[LOGGER_BUFFER_SIZE] = {0};
         size_t loggerBufferCurrentPos         = 0;
         size_t maxFileSize                    = MAX_LOG_FILE_SIZE;
-        int currentRotationIndex              = 0;
-        int maxRotationIndex                  = 0;
 
         Application application;
         LoggerBuffer circularBuffer;
+        utils::Rotator<MAX_LOG_FILES_COUNT> rotator;
         static constexpr size_t circularBufferSize = 1000;
 
         static const char *levelNames[];

@@ 20,6 20,7 @@ add_catch2_executable(
     LIBS
         module-utils
         module-vfs
+        log    
     USE_FS
 )
 

@@ 38,7 38,6 @@ TEST_CASE("Test if logs are dumped to a file without rotation")
 {
     auto app                            = Log::Application{"TestApp", "rev", "tag", "branch"};
     constexpr auto MaxFileSize          = 1024 * 1024; // 1 MB
-    constexpr auto MaxFilesCount        = 3;
     constexpr auto TestLog              = "12345678";
     const std::filesystem::path logsDir = "./ut_logs";
     const auto testLogFile              = logsDir / "TestApp.log";


@@ 50,7 49,7 @@ TEST_CASE("Test if logs are dumped to a file without rotation")
     std::filesystem::create_directory(logsDir);
 
     // Initialize the logger with test parameters.
-    Log::Logger::get().init(std::move(app), MaxFileSize, MaxFilesCount);
+    Log::Logger::get().init(std::move(app), MaxFileSize);
     REQUIRE(countFiles(logsDir) == 0);
 
     // Dump logs.


@@ 72,7 71,6 @@ TEST_CASE("Test if log files rotate")
 {
     auto app                            = Log::Application{"TestApp", "rev", "tag", "branch"};
     constexpr auto MaxFileSize          = 10; // 10 bytes
-    constexpr auto MaxFilesCount        = 3;
     constexpr auto TestLog              = "12345678";
     const std::filesystem::path logsDir = "./ut_logs";
     const auto testLogFile              = logsDir / "TestApp.log";


@@ 84,7 82,7 @@ TEST_CASE("Test if log files rotate")
     std::filesystem::create_directory(logsDir);
 
     // Initialize the logger with test parameters.
-    Log::Logger::get().init(std::move(app), MaxFileSize, MaxFilesCount);
+    Log::Logger::get().init(std::move(app), MaxFileSize);
     REQUIRE(countFiles(logsDir) == 0);
 
     // Dump logs.

@@ 0,0 1,15 @@
+add_library(utils-rotator INTERFACE)
+
+target_sources(utils-rotator
+    PUBLIC
+        include/rotator/Rotator.hpp
+)
+
+target_include_directories(utils-rotator
+    INTERFACE
+        $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>
+)
+
+if (${ENABLE_TESTS})
+    add_subdirectory(tests)
+endif()

@@ 0,0 1,82 @@
+// Copyright (c) 2017-2021, Mudita Sp. z.o.o. All rights reserved.
+// For licensing, see https://github.com/mudita/MuditaOS/LICENSE.md
+
+#pragma once
+
+#include <filesystem>
+#include <string>
+
+namespace utils
+{
+    template <std::size_t maxRotationFilesCount> class Rotator
+    {
+      protected:
+        const std::string extension;
+
+        std::size_t getRotatedFilesCount(const std::filesystem::path &path, std::error_code &ec) const
+        {
+            std::size_t count = 0;
+            for (std::size_t i = 1; i < maxRotationFilesCount; i++) {
+                if (!std::filesystem::exists(getRotatedFilePath(path, i), ec)) {
+                    break;
+                }
+                if (ec) {
+                    break;
+                }
+                count++;
+            }
+            return count;
+        }
+
+        std::string getRotatedFileExtension(int count) const
+        {
+            return extension + "." + std::to_string(count);
+        }
+
+        std::filesystem::path getRotatedFilePath(const std::filesystem::path &source, int rotationCount) const
+        {
+            auto path = source;
+            path.replace_extension(getRotatedFileExtension(rotationCount));
+            return path;
+        }
+
+      public:
+        explicit Rotator(std::string extension) : extension{extension}
+        {}
+
+        virtual ~Rotator() = default;
+
+        bool rotateFile(const std::filesystem::path &path) const
+        {
+            std::error_code ec;
+            if (const auto firstDump = !std::filesystem::exists(path, ec); ec) {
+                return false;
+            }
+            else if (firstDump) {
+                return true;
+            }
+            const auto rotatedFiles = getRotatedFilesCount(path, ec);
+            if (ec) {
+                return false;
+            }
+            for (std::size_t i = rotatedFiles; i > 0; i--) {
+                const auto src = getRotatedFilePath(path, i);
+                if (i == maxRotationFilesCount - 1) {
+                    std::filesystem::remove(src, ec);
+                    if (ec) {
+                        return false;
+                    }
+                    continue;
+                }
+                const auto dest = getRotatedFilePath(path, i + 1);
+                std::filesystem::rename(src, dest, ec);
+                if (ec) {
+                    return false;
+                }
+            }
+            auto rotatedLogPath = getRotatedFilePath(path, 1);
+            std::filesystem::rename(path, rotatedLogPath, ec);
+            return (ec) ? false : true;
+        }
+    };
+} // namespace utils

@@ 0,0 1,9 @@
+# Rotator tests
+add_catch2_executable(
+    NAME
+        rotator-test
+    SRCS
+        test_Rotator.cpp
+    LIBS
+        utils-rotator
+)

@@ 0,0 1,94 @@
+// Copyright (c) 2017-2021, Mudita Sp. z.o.o. All rights reserved.
+// For licensing, see https://github.com/mudita/MuditaOS/LICENSE.md
+
+#define CATCH_CONFIG_MAIN
+
+#include <catch2/catch.hpp>
+#include <rotator/Rotator.hpp>
+
+#include <iostream>
+#include <filesystem>
+
+namespace
+{
+    constexpr std::size_t maxRotationFilesCount = 5;
+
+    class HexRotator : public utils::Rotator<maxRotationFilesCount>
+    {
+      public:
+        HexRotator() : utils::Rotator<maxRotationFilesCount>{".hex"}
+        {}
+
+        std::filesystem::path rotatedFilePath(const std::filesystem::path &source, int rotationCount)
+        {
+            return this->getRotatedFilePath(source, rotationCount);
+        }
+    };
+} // namespace
+
+TEST_CASE("Rotation of hex dump files")
+{
+    HexRotator rotator;
+    constexpr auto fileName = "crashdump.hex";
+    SECTION("Handle first dump")
+    {
+        REQUIRE(rotator.rotateFile(fileName));
+    }
+    SECTION("Handle first rotate")
+    {
+        constexpr auto firstRotationName = "crashdump.hex.1";
+        std::ofstream file(fileName);
+        REQUIRE(rotator.rotateFile(fileName));
+        REQUIRE(std::filesystem::exists(firstRotationName));
+        REQUIRE_FALSE(std::filesystem::exists(fileName));
+        REQUIRE(std::filesystem::remove(firstRotationName));
+    }
+    SECTION("Handle max rotates")
+    {
+        for (std::size_t i = 1; i <= maxRotationFilesCount; i++) {
+            const auto rotatedFileName = rotator.rotatedFilePath(fileName, i);
+            std::ofstream file(fileName);
+            REQUIRE(rotator.rotateFile(fileName));
+            for (std::size_t j = i - 1; j > 0; j--) {
+                REQUIRE(std::filesystem::exists(rotator.rotatedFilePath(fileName, j)));
+            }
+            if (i == maxRotationFilesCount) {
+                REQUIRE_FALSE(std::filesystem::exists(rotator.rotatedFilePath(fileName, i)));
+            }
+            REQUIRE_FALSE(std::filesystem::exists(fileName));
+        }
+        for (std::size_t i = 1; i < maxRotationFilesCount; i++) {
+            const auto rotatedFileName = rotator.rotatedFilePath(fileName, i);
+            REQUIRE(std::filesystem::remove(rotatedFileName));
+        }
+    }
+
+    SECTION("Handle more than max files")
+    {
+        for (std::size_t i = 1; i <= maxRotationFilesCount; i++) {
+            const auto rotatedFileName = rotator.rotatedFilePath(fileName, i);
+            std::ofstream file(fileName);
+            REQUIRE(rotator.rotateFile(fileName));
+            for (std::size_t j = i - 1; j > 0; j--) {
+                REQUIRE(std::filesystem::exists(rotator.rotatedFilePath(fileName, j)));
+            }
+            if (i == maxRotationFilesCount) {
+                REQUIRE_FALSE(std::filesystem::exists(rotator.rotatedFilePath(fileName, i)));
+            }
+            REQUIRE_FALSE(std::filesystem::exists(fileName));
+        }
+        std::ofstream file(fileName);
+        REQUIRE(rotator.rotateFile(fileName));
+        REQUIRE_FALSE(std::filesystem::exists(rotator.rotatedFilePath(fileName, maxRotationFilesCount + 1)));
+        for (std::size_t i = 1; i < maxRotationFilesCount; i++) {
+            const auto rotatedFileName = rotator.rotatedFilePath(fileName, i);
+            REQUIRE(std::filesystem::exists(rotatedFileName));
+        }
+        REQUIRE_FALSE(std::filesystem::exists(rotator.rotatedFilePath(fileName, maxRotationFilesCount)));
+        REQUIRE_FALSE(std::filesystem::exists(fileName));
+        for (std::size_t i = 1; i < maxRotationFilesCount; i++) {
+            const auto rotatedFileName = rotator.rotatedFilePath(fileName, i);
+            REQUIRE(std::filesystem::remove(rotatedFileName));
+        }
+    }
+}