Refactor Os::FileSystem and Os::Directory

Os/CMakeLists.txt

@@ -35,9 +35,12 @@ set(SOURCE_FILES
     "${CMAKE_CURRENT_LIST_DIR}/ValidatedFile.cpp"
 
     # Refactored common files
+    "${CMAKE_CURRENT_LIST_DIR}/Os.cpp"
     "${CMAKE_CURRENT_LIST_DIR}/File.cpp"
     "${CMAKE_CURRENT_LIST_DIR}/Task.cpp"
     "${CMAKE_CURRENT_LIST_DIR}/Mutex.cpp"
+    "${CMAKE_CURRENT_LIST_DIR}/FileSystem.cpp"
+    "${CMAKE_CURRENT_LIST_DIR}/Directory.cpp"
 )
 # Check for default logger
 if (NOT FPRIME_DISABLE_DEFAULT_LOGGER)
@@ -56,8 +59,6 @@ if (FPRIME_USE_POSIX)
         "${CMAKE_CURRENT_LIST_DIR}/Linux/InterruptLock.cpp"
         "${CMAKE_CURRENT_LIST_DIR}/Linux/WatchdogTimer.cpp"
         "${CMAKE_CURRENT_LIST_DIR}/Posix/IntervalTimer.cpp"
-        "${CMAKE_CURRENT_LIST_DIR}/Linux/FileSystem.cpp"
-        "${CMAKE_CURRENT_LIST_DIR}/Linux/Directory.cpp"
     )
 endif()
 # Darwin IPC queue implementation
@@ -97,26 +98,34 @@ endif()
 register_fprime_module()
 
 require_fprime_implementation(Os/File)
+require_fprime_implementation(Os/FileSystem)
+require_fprime_implementation(Os/Directory)
+require_fprime_implementation(Os/Mutex)
 # require_fprime_implementation(Os/Task) # should be added in
-require_fprime_implementation(Os/Mutex) 
+# require_fprime_implementation(Os/Console) # should be added in
 
 
 ### UTS ### Note: 3 separate UTs registered here.
 set(UT_SOURCE_FILES
   "${CMAKE_CURRENT_LIST_DIR}/test/ut/OsQueueTest.cpp"
   "${CMAKE_CURRENT_LIST_DIR}/test/ut/OsTestMain.cpp"
   "${CMAKE_CURRENT_LIST_DIR}/test/ut/IntervalTimerTest.cpp"
-  "${CMAKE_CURRENT_LIST_DIR}/test/ut/OsFileSystemTest.cpp"
+  # "${CMAKE_CURRENT_LIST_DIR}/test/ut/OsFileSystemTest.cpp"
   "${CMAKE_CURRENT_LIST_DIR}/test/ut/OsValidateFileTest.cpp"
   "${CMAKE_CURRENT_LIST_DIR}/test/ut/OsSystemResourcesTest.cpp"
   "${CMAKE_CURRENT_LIST_DIR}/test/ut/OsMutexBasicLockableTest.cpp"
 )
 register_fprime_ut()
 if (BUILD_TESTING)
-    foreach (TEST IN ITEMS StubFileTest PosixFileTest StubMutexTest PosixMutexTest) # add? : StubTaskTest PosixTaskTest
-       if (TARGET "${TEST}")
-           add_dependencies("Os_ut_exe" "${TEST}")
-       endif()
+    foreach (TEST IN ITEMS 
+                StubFileTest PosixFileTest
+                StubMutexTest PosixMutexTest
+                StubDirectoryTest PosixDirectory
+                StubFileSystemTest PosixFileSystemTest
+            ) # add? : StubTaskTest PosixTaskTest
+        if (TARGET "${TEST}")
+            add_dependencies("Os_ut_exe" "${TEST}")
+        endif()
     endforeach()
 endif()
 # Second UT Pthreads

Os/Directory.cpp

@@ -0,0 +1,126 @@
+// ======================================================================
+// \title Os/Directory.cpp
+// \brief common function implementation for Os::Directory
+// ======================================================================
+#include <Fw/Types/Assert.hpp>
+#include <Os/Directory.hpp>
+
+namespace Os {
+
+Directory::Directory() : m_handle_storage(), m_delegate(*DirectoryInterface::getDelegate(m_handle_storage)) {
+    FW_ASSERT(&this->m_delegate == reinterpret_cast<DirectoryInterface*>(&this->m_handle_storage[0]));
+}
+
+Directory::~Directory() {
+    FW_ASSERT(&this->m_delegate == reinterpret_cast<DirectoryInterface*>(&this->m_handle_storage[0]));
+    m_delegate.~DirectoryInterface();
+}
+
+DirectoryHandle* Directory::getHandle() {
+    FW_ASSERT(&this->m_delegate == reinterpret_cast<DirectoryInterface*>(&this->m_handle_storage[0]));
+    return this->m_delegate.getHandle();
+}
+
+Directory::Status Directory::open(const char* path, OpenMode mode) {
+    FW_ASSERT(&this->m_delegate == reinterpret_cast<DirectoryInterface*>(&this->m_handle_storage[0]));
+    return this->m_delegate.open(path, mode);
+}
+
+bool Directory::isOpen() {
+    FW_ASSERT(&this->m_delegate == reinterpret_cast<DirectoryInterface*>(&this->m_handle_storage[0]));
+    return this->m_delegate.isOpen();
+}
+Directory::Status Directory::rewind() {
+    FW_ASSERT(&this->m_delegate == reinterpret_cast<DirectoryInterface*>(&this->m_handle_storage[0]));
+    if (!this->m_delegate.isOpen()) {
+        return Status::NOT_OPENED;
+    }
+    return this->m_delegate.rewind();
+}
+
+Directory::Status Directory::read(char * fileNameBuffer, U32 bufSize) {
+    FW_ASSERT(&this->m_delegate == reinterpret_cast<DirectoryInterface*>(&this->m_handle_storage[0]));
+    if (!this->m_delegate.isOpen()) {
+        return Status::NOT_OPENED;
+    }
+    return this->m_delegate.read(fileNameBuffer, bufSize);
+}
+
+void Directory::close() {
+    FW_ASSERT(&this->m_delegate == reinterpret_cast<DirectoryInterface*>(&this->m_handle_storage[0]));
+    return this->m_delegate.close();
+}
+
+// ------------ Common Directory Functions (non-OS-specific) ------------
+
+Directory::Status Directory::getFileCount(FwSizeType& fileCount) {
+    if (this->isOpen() == false) {
+        return Status::NOT_OPENED;
+    }
+    // REVIEW NOTE: should getFileCount rewind before counting?
+    // Likely yes?? Otherwise it's just weird
+    // TODO: check return value (how to handle if not OP_OK?)
+    this->rewind();
+
+    const U32 loopLimit = std::numeric_limits<U32>::max();
+    FwSizeType count = 0;
+    FwSizeType unusedBufferSize = 1;
+    char unusedBuffer[unusedBufferSize];
+    Status readStatus = Status::OP_OK;
+    for (U32 iter = 0; iter < loopLimit; ++iter) {
+        readStatus = this->read(unusedBuffer, unusedBufferSize);
+        if (readStatus == Status::NO_MORE_FILES) {
+            break;
+        } else if (readStatus != Status::OP_OK) {
+            return Status::OTHER_ERROR;
+        }
+        ++count;
+    }
+    fileCount = count;
+    // and after??
+    this->rewind();
+    return Status::OP_OK;
+}
+
+
+Directory::Status Directory::readDirectory(Fw::String filenameArray[], const FwSizeType filenameArraySize, FwSizeType& filenameCount) {
+    FW_ASSERT(filenameArray != nullptr);
+    FW_ASSERT(filenameArraySize > 0);
+    if (this->isOpen() == false) {
+        return Status::NOT_OPENED;
+    }
+    // same thing here - should we rewind before?
+    this->rewind();
+
+    Status readStatus = Status::OP_OK;
+    Status returnStatus = Status::OP_OK;
+
+    FwIndexType index;
+    constexpr FwIndexType loopLimit = std::numeric_limits<FwIndexType>::max();
+
+    char fileName[FPP_CONFIG_FILENAME_MAX_SIZE]; // should be FW_FIXED_LENGTH_STRING_SIZE instead??
+
+    for (index = 0; ((index < loopLimit) && (index < static_cast<FwIndexType>(filenameArraySize))); index++) {
+        readStatus = this->read(fileName, FPP_CONFIG_FILENAME_MAX_SIZE);
+        if (readStatus == Status::NO_MORE_FILES) {
+            break;
+        } else if (readStatus != Status::OP_OK) {
+            return Status::OTHER_ERROR;
+        }
+
+        filenameArray[index] = Fw::String(fileName);
+    }
+
+    filenameCount = index;
+
+    if (index == loopLimit) {
+        returnStatus = Status::FILE_LIMIT;
+    }
+    this->rewind();
+
+    return returnStatus;
+
+}
+
+
+}  // namespace Os

Os/Directory.hpp

@@ -1,44 +1,122 @@
-#ifndef _Directory_hpp_
-#define _Directory_hpp_
+#ifndef _OS_DIRECTORY_HPP_
+#define _OS_DIRECTORY_HPP_
 
 #include <FpConfig.hpp>
+#include <Os/Os.hpp>
+#include <config/FppConstantsAc.hpp>
+#include <Fw/Types/String.hpp>
 
 namespace Os {
 
-    // This class encapsulates a very simple directory interface that has the most often-used features
+struct DirectoryHandle {};
 
-    class Directory {
-        public:
+class DirectoryInterface {
+  public:
+    static constexpr FwSizeType FPP_CONFIG_FILENAME_MAX_SIZE = FppConstant_FileNameStringSize::FileNameStringSize;
+    typedef enum {
+        OP_OK, //!<  Operation was successful
+        DOESNT_EXIST, //!<  Directory doesn't exist
+        NO_PERMISSION, //!<  No permission to read directory
+        NOT_OPENED, //!<  Directory hasn't been opened yet
+        NOT_DIR, //!<  Path is not a directory
+        NO_MORE_FILES, //!<  Directory stream has no more files
+        FILE_LIMIT, //!<  Directory has more files than can be read
+        BAD_DESCRIPTOR, //!<  Directory stream descriptor is invalid
+        NOT_SUPPORTED, //!<  Operation is not supported by the current implementation
+        OTHER_ERROR, //!<  A catch-all for other errors. Have to look in implementation-specific code
+    } Status;
 
-            typedef enum {
-                OP_OK, //!<  Operation was successful
-                DOESNT_EXIST, //!<  Directory doesn't exist
-                NO_PERMISSION, //!<  No permission to read directory
-                NOT_OPENED, //!<  Directory hasn't been opened yet
-                NOT_DIR, //!<  Path is not a directory
-                NO_MORE_FILES, //!<  Directory stream has no more files
-                OTHER_ERROR, //!<  A catch-all for other errors. Have to look in implementation-specific code
-            } Status;
+    typedef enum {
+        READ,   //!<  Error if directory doesn't exist
+        CREATE, //!<  Create directory if it doesn't exist
+    } OpenMode;
 
-            Directory(); //!<  Constructor
-            virtual ~Directory(); //!<  Destructor. Will close directory if still open
-            Status open(const char* dirName); //!<  open directory. Directory must already exist
-            bool isOpen(); //!< check if file descriptor is open or not.
-            Status rewind(); //!<  rewind directory stream to the beginning
+    //! \brief default constructor
+    DirectoryInterface() = default;
 
-            Status read(char * fileNameBuffer, U32 bufSize); //!< get next filename from directory
-            Status read(char * fileNameBuffer, U32 bufSize, I64& inode); //!< get next filename and inode from directory
-            void close(); //!<  close directory
+    //! \brief default virtual destructor
+    virtual ~DirectoryInterface() = default;
 
-            NATIVE_INT_TYPE getLastError(); //!< read back last error code (typically errno)
-            const char* getLastErrorString(); //!< get a string of the last error (typically from strerror)
+    //! \brief copy constructor is forbidden
+    DirectoryInterface(const DirectoryInterface& other) = delete;
 
-        private:
+    //! \brief assignment operator is forbidden
+    DirectoryInterface& operator=(const DirectoryInterface& other) = delete;
 
-            POINTER_CAST m_dir; //!<  Stored directory pointer (type varies across implementations)
-            NATIVE_INT_TYPE m_lastError; //!<  stores last error
+    //! \brief return the underlying Directory handle (implementation specific)
+    //! \return internal Directory handle representation
+    virtual DirectoryHandle* getHandle() = 0;
 
-    };
+    //! \brief provide a pointer to a Directory delegate object
+    static DirectoryInterface* getDelegate(HandleStorage& aligned_new_memory);
+
+
+    //------------ Os-specific Directory Functions ------------
+
+    virtual Status open(const char* path, OpenMode mode) = 0; //!<  open/create a directory
+    virtual bool isOpen() = 0; //!< check if file descriptor is open or not.
+    virtual Status rewind() = 0; //!<  rewind directory stream to the beginning
+    virtual Status read(char * fileNameBuffer, U32 bufSize) = 0; //!< get next filename from directory
+    virtual void close() = 0; //!<  close directory
+
+};
+
+class Directory final : public DirectoryInterface {
+  public:
+    Directory();         //!<  Constructor (private because singleton pattern)
+    ~Directory() final;  //!<  Destructor
+
+    //! \brief return the underlying Directory handle (implementation specific)
+    //! \return internal Directory handle representation
+    DirectoryHandle* getHandle() override;
+
+    //------------ Os-specific Directory Functions ------------
+
+    //! \brief Open or create a directory
+    //! \param path: path of directory to open
+    //! \param mode: enum (READ, CREATE). READ will return an error if directory doesn't exist
+    //! \return status of the operation
+    Status open(const char* path, OpenMode mode) override;
+
+    //! \brief Check if Directory is open or not
+    //! \return true if Directory is open, false otherwise
+    bool isOpen() override;
+
+    //! \brief Rewind directory stream to the beginning
+    //! \return status of the operation
+    Status rewind() override;
+
+    //! \brief Get next filename from directory stream
+    //! \param fileNameBuffer: buffer to store filename
+    //! \param bufSize: size of fileNameBuffer
+    //! \return status of the operation
+    Status read(char * fileNameBuffer, U32 bufSize) override;
+
+    //! \brief Close directory
+    void close() override;
+
+    // ------------ Common Directory Functions (non-OS-specific) ------------
+
+    //! \brief Read the contents of the directory and store filenames in filenameArray of size arraySize
+    //! \param filenameArray: array to store filenames
+    //! \param arraySize: size of filenameArray
+    //! \param filenameCount: number of filenames written to filenameArray (output)
+    //! \return status of the operation
+    Status readDirectory(Fw::String filenameArray[], const FwSizeType arraySize, FwSizeType& filenameCount);
+
+    //! \brief Get the number of files in the directory
+    //! \param fileCount: number of files in the directory (output)
+    //! \return status of the operation
+    Status getFileCount(FwSizeType& fileCount);
+
+  private:
+    // This section is used to store the implementation-defined Directory handle. To Os::Directory and fprime, this type is
+    // opaque and thus normal allocation cannot be done. Instead, we allow the implementor to store then handle in
+    // the byte-array here and set `handle` to that address for storage.
+    //
+    alignas(FW_HANDLE_ALIGNMENT) HandleStorage m_handle_storage;  //!< Directory handle storage
+    DirectoryInterface& m_delegate;          
+};
 
 }