From f99b2f63c12301a94733523c7d9fbeda5149ee4e Mon Sep 17 00:00:00 2001
From: Mark Olesen <Mark.Olesen@esi-group.com>
Date: Mon, 13 May 2024 14:31:32 +0200
Subject: [PATCH] ENH: improve OFstream append behaviour (#3160)

- previous support for file appending (unused in the meantime)
  specified opening with `std::ios_base::app`. However, this also
  enforces append behaviour for each write operation and thus
  disallows any seek/repositioning within the output file.

  Now treat append as an "append-like" behaviour instead.

  If the file already exists, its contents will be preserved and
  the *initial* output position is moved to the file end. All
  subsequent write operations respect the current file position
  without an additional seek-to-end on each write. This enables
  support of file appending/overwriting, but does preclude concurrent
  file output.
---
 applications/test/OFstream/Test-OFstream.cxx  | 166 +++++++++++++++---
 src/OpenFOAM/db/IOstreams/Fstreams/OFstream.H |  42 ++++-
 .../db/IOstreams/Fstreams/fstreamPointer.H    |  42 ++++-
 .../db/IOstreams/Fstreams/fstreamPointers.C   | 103 ++++++++---
 4 files changed, 291 insertions(+), 62 deletions(-)

diff --git a/applications/test/OFstream/Test-OFstream.cxx b/applications/test/OFstream/Test-OFstream.cxx
index ac108699b0f..bab6fc22cdd 100644
--- a/applications/test/OFstream/Test-OFstream.cxx
+++ b/applications/test/OFstream/Test-OFstream.cxx
@@ -5,7 +5,7 @@
     \\  /    A nd           | www.openfoam.com
      \\/     M anipulation  |
 -------------------------------------------------------------------------------
-    Copyright (C) 2022 OpenCFD Ltd.
+    Copyright (C) 2022-2024 OpenCFD Ltd.
 -------------------------------------------------------------------------------
 License
     This file is part of OpenFOAM.
@@ -32,10 +32,14 @@ Description
 #include "IOstreams.H"
 #include "OSspecific.H"
 #include "argList.H"
+#include "clock.H"
+#include "Switch.H"
 #include "ListOps.H"
 
 using namespace Foam;
 
+std::string time_stamp;
+
 void listFiles(const fileName& dir)
 {
     wordList files = ListOps::create<word>
@@ -55,6 +59,41 @@ void listFiles(const fileName& dir)
 }
 
 
+OSstream& printInfo(OFstream& os)
+{
+    InfoErr
+        << "open: " << os.name() << nl
+        << "is_append: " << Switch::name(os.is_append())
+        << " tellp: "<< os.stdStream().tellp() << nl;
+
+    return InfoErr.stream();
+}
+
+
+void withHeader(OFstream& os)
+{
+    const auto tellp = os.stdStream().tellp();
+
+    if (tellp == 0)
+    {
+        InfoErr
+            << "Add header" << nl;
+        os  << "HEADER: " << time_stamp.c_str() << nl;
+    }
+}
+
+
+template<class OSstreamType>
+void generateLines(OSstreamType& os, label count = 1)
+{
+    for (label line = 1; line <= count; ++line)
+    {
+        os  << "[" << line
+            << "] =============================================" << nl;
+    }
+}
+
+
 // * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * //
 // Main program:
 
@@ -62,18 +101,31 @@ int main(int argc, char *argv[])
 {
     argList::addBoolOption("gz", "Use compression");
     argList::addBoolOption("append", "Use append mode");
+    argList::addBoolOption("seekend", "Seek to end after non-append open");
+    argList::addOption("seek", "value", "Seek from start (default: 100)");
     argList::addBoolOption("atomic", "Use atomic");
     argList::addBoolOption("keep", "Do not remove test directory");
     argList::addOption("write", "file", "test writing to file");
 
     #include "setRootCase.H"
 
+    // Same time-stamp for all generated files
+    time_stamp = clock::dateTime();
+
     const fileName baseDir("Test-OFstream-directory");
 
     Foam::mkDir(baseDir);
 
     InfoErr<< "mkdir: " << baseDir << endl;
 
+    Info<< "start:" << nl;
+    listFiles(baseDir);
+
+    const bool with_append = args.found("append");
+    const bool with_seekend = args.found("seekend");
+
+    const int seek_out = args.getOrDefault<int>("seek", 100);
+
     IOstreamOption streamOpt;
 
     if (args.found("gz"))
@@ -97,7 +149,6 @@ int main(int argc, char *argv[])
     {
         OFstream(baseDir/"dummy")() << "Some file content" << endl;
 
-        Foam::ln("dummy", baseDir/"Test2.txt");
         Foam::ln("dummy", baseDir/"Test3.txt");
         Foam::ln("dummy", baseDir/"Test4.txt");
         Foam::ln("dummy", baseDir/"Test4.txt.gz");
@@ -114,16 +165,58 @@ int main(int argc, char *argv[])
             append
         );
 
-        os << "=========================" << endl;
+        if (with_seekend && !with_append)
+        {
+            os.stdStream().seekp(0, std::ios_base::end);
+        }
+
+        printInfo(os);
+
+        withHeader(os);
 
-        InfoErr<< "open: " << os.name() << endl;
-        InfoErr<< "... sleep" << endl;
+        {
+            InfoErr<< "... seekp(" << seek_out << ")" << nl;
+
+            auto& oss = os.stdStream();
+
+            // Actually std::streampos, but cannot increment that
+
+            int64_t pos(seek_out);
+
+            const int64_t tellp_end = oss.tellp();
+
+            if (pos >= 0 && pos < tellp_end)
+            {
+                InfoErr
+                    << "... fill from " << label(pos)
+                    << " to " << label(tellp_end) << nl;
+
+                oss.seekp(pos);
+
+                while (pos < tellp_end)
+                {
+                    // Fill with char 'X', rely on streambuf buffering
+                    oss << 'X';
+                    ++pos;
+                }
+
+                oss.seekp(seek_out);
+                os << "More content [at " << seek_out << ']' << endl;
+            }
+        }
+
+        generateLines(os, 4);
+
+        printInfo(os)
+            << "... sleep" << endl;
 
         listFiles(baseDir);
 
         sleep(2);
 
-        os << "+++++++++++++++++++++++++++++++++++" << endl;
+        os << "[new content] +++++++++++++++++++++++++++++++++++" << endl;
+
+
     }
 
     {
@@ -135,16 +228,24 @@ int main(int argc, char *argv[])
             // NON_APPEND
         );
 
-        os << "=========================" << endl;
+        if (with_seekend)  // NON_APPEND
+        {
+            os.stdStream().seekp(0, std::ios_base::end);
+        }
+        printInfo(os);
 
-        InfoErr<< "open: " << os.name() << endl;
-        InfoErr<< "... sleep" << endl;
+        withHeader(os);
+
+        generateLines(os, 4);
+
+        printInfo(os)
+            << "... sleep" << endl;
 
         listFiles(baseDir);
 
         sleep(2);
 
-        os << "+++++++++++++++++++++++++++++++++++" << endl;
+        os << "[new content] +++++++++++++++++++++++++++++++++++" << endl;
     }
     {
         OFstream os
@@ -155,16 +256,26 @@ int main(int argc, char *argv[])
             IOstreamOption::APPEND
         );
 
-        os << "=========================" << endl;
+        // No seekend with APPEND, but try anyhow
+        if (with_seekend)
+        {
+            os.stdStream().seekp(0, std::ios_base::end);
+        }
+
+        printInfo(os);
 
-        InfoErr<< "open: " << os.name() << endl;
-        InfoErr<< "... sleep" << endl;
+        withHeader(os);
+
+        generateLines(os, 4);
+
+        printInfo(os)
+            << "... sleep" << endl;
 
         listFiles(baseDir);
 
         sleep(2);
 
-        os << "+++++++++++++++++++++++++++++++++++" << endl;
+        os << "[new content] +++++++++++++++++++++++++++++++++++" << endl;
     }
     {
         OFstream os
@@ -174,10 +285,16 @@ int main(int argc, char *argv[])
             IOstreamOption::COMPRESSED
         );
 
-        os << "=========================" << endl;
+        // No seekend with COMPRESSED
 
-        InfoErr<< "open: " << os.name() << endl;
-        InfoErr<< "... sleep" << endl;
+        printInfo(os);
+
+        withHeader(os);
+
+        generateLines(os, 4);
+
+        printInfo(os)
+            << "... sleep" << endl;
 
         listFiles(baseDir);
 
@@ -193,10 +310,19 @@ int main(int argc, char *argv[])
             // ASCII UNCOMPRESSED NON_APPEND
         );
 
-        os << "=========================" << endl;
+        if (with_seekend)  // NON_APPEND
+        {
+            os.stdStream().seekp(0, std::ios_base::end);
+        }
+
+        printInfo(os);
+
+        withHeader(os);
+
+        generateLines(os, 4);
 
-        InfoErr<< "open: " << os.name() << endl;
-        InfoErr<< "... sleep" << endl;
+        printInfo(os)
+            << "... sleep" << endl;
 
         listFiles(baseDir);
 
diff --git a/src/OpenFOAM/db/IOstreams/Fstreams/OFstream.H b/src/OpenFOAM/db/IOstreams/Fstreams/OFstream.H
index ca0819ff430..767406e8b99 100644
--- a/src/OpenFOAM/db/IOstreams/Fstreams/OFstream.H
+++ b/src/OpenFOAM/db/IOstreams/Fstreams/OFstream.H
@@ -6,7 +6,7 @@
      \\/     M anipulation  |
 -------------------------------------------------------------------------------
     Copyright (C) 2011-2017 OpenFOAM Foundation
-    Copyright (C) 2017-2023 OpenCFD Ltd.
+    Copyright (C) 2017-2024 OpenCFD Ltd.
 -------------------------------------------------------------------------------
 License
     This file is part of OpenFOAM.
@@ -28,7 +28,22 @@ Class
     Foam::OFstream
 
 Description
-    Output to file stream, using an OSstream
+    Output to file stream as an OSstream, normally using std::ofstream
+    for the actual output.
+
+Note
+    The atomic output works by creating an intermediate temporary file,
+    which is renamed as an atomic operation when closing. It is not
+    possible, or particularly desirable, to have an atomic in combination
+    with append behaviour. If both are specified, append has priority.
+
+    When an output file is opened in \c append mode, it has "append-like"
+    behaviour, which means that existing file content will be preserved
+    and the \em initial output position is moved to the file end.
+
+    Subsequent write operations respect the current file position without an
+    additional seek-to-end on each write. This enables support of file
+    overwriting, but precludes concurrent file output.
 
 SourceFiles
     OFstream.C
@@ -64,12 +79,11 @@ public:
 
     // Constructors
 
-        //- Construct a null output file stream.
-        //  Behaves like \c /dev/null and is named accordingly
+        //- Construct a null output file stream that behaves like \c /dev/null
         explicit OFstream(std::nullptr_t);
 
         //- Construct with specified atomic behaviour
-        //- from pathname, stream option, optional append
+        //- from pathname, stream option, optional append (see note).
         OFstream
         (
             IOstreamOption::atomicType atomic,
@@ -78,7 +92,8 @@ public:
             IOstreamOption::appendType append = IOstreamOption::NON_APPEND
         );
 
-        //- Construct from pathname and other specifications
+        //- Construct from pathname and other specifications.
+        //  See note on append mode.
         explicit OFstream
         (
             const fileName& pathname,
@@ -89,7 +104,8 @@ public:
             OFstream(IOstreamOption::NON_ATOMIC, pathname, streamOpt, append)
         {}
 
-        //- Construct from pathname, format (uncompressed), optional append,
+        //- Construct from pathname, format (uncompressed),
+        //- optional append (see note),
         //- atomic behaviour as per system default
         OFstream
         (
@@ -103,7 +119,8 @@ public:
         {}
 
         //- Construct with specified atomic behaviour
-        //- from pathname, format (uncompressed), optional append
+        //- from pathname, format (uncompressed),
+        //- optional append (see note).
         OFstream
         (
             IOstreamOption::atomicType atomic,
@@ -141,6 +158,15 @@ public:
         virtual void rewind();
 
 
+    // Output stream modes
+
+        //- True if file already existed \em and was opened in append-like mode
+        bool is_append() const noexcept { return ofstreamPointer::is_append(); }
+
+        //- True if file creation behaves as atomic
+        bool is_atomic() const noexcept { return ofstreamPointer::is_atomic(); }
+
+
     // Print
 
         //- Print stream description
diff --git a/src/OpenFOAM/db/IOstreams/Fstreams/fstreamPointer.H b/src/OpenFOAM/db/IOstreams/Fstreams/fstreamPointer.H
index e3c98907936..e18ef8b9abf 100644
--- a/src/OpenFOAM/db/IOstreams/Fstreams/fstreamPointer.H
+++ b/src/OpenFOAM/db/IOstreams/Fstreams/fstreamPointer.H
@@ -194,20 +194,31 @@ public:
 
 class ofstreamPointer
 {
+    // Private Data Types
+
+        //- The file open/creation type (bitmask)
+        enum modeType : char
+        {
+            NONE = 0,
+            ATOMIC = 0x1,
+            APPEND = 0x2
+        };
+
+
     // Private Data
 
         //- The stream pointer (ofstream | ogzstream | ocountstream, ...)
         std::unique_ptr<std::ostream> ptr_;
 
-        //- Atomic file creation
-        bool atomic_;
+        //- File output/creation type (atomic, append etc)
+        char mode_;
 
 
 protected:
 
     // Protected Member Functions
 
-        //- Reopen for compressed/non-compressed
+        //- Reopen for compressed/non-compressed. Discards append status.
         void reopen(const std::string& pathname);
 
         //- Close stream and rename file
@@ -249,12 +260,17 @@ public:
         //  \param atomic   Write into temporary file (not target file).
         //      This option should only be used with a stream wrapper
         //      (eg, OFstream) that handles the final renaming.
+        //
+        //  \note The append mode is actually 'append-like', which means
+        //        that it preserves existing files and moves the \em initial
+        //        position to the end, but does not enforce seeking to the end
+        //        for every write operation.
         explicit ofstreamPointer
         (
             const fileName& pathname,
             IOstreamOption streamOpt = IOstreamOption(),
-            const bool append = false,
-            const bool atomic = false
+            bool append = false,
+            bool atomic = false
         );
 
         //- Construct from pathname, compression, append, file handling atomic
@@ -268,12 +284,12 @@ public:
         (
             const fileName& pathname,
             IOstreamOption::compressionType comp,
-            const bool append = false,
-            const bool atomic = false
+            bool append = false,
+            bool atomic = false
         );
 
 
-    // Member Functions
+    // Static Functions
 
         //- True if compiled with libz support
         static bool supports_gz() noexcept;
@@ -293,13 +309,21 @@ public:
         //- Which compression type?
         IOstreamOption::compressionType whichCompression() const;
 
+        //- True if file already existed \em and was opened in append-like mode
+        bool is_append() const noexcept { return (mode_ & modeType::APPEND); }
+
+        //- True if file creation behaves as atomic
+        bool is_atomic() const noexcept { return (mode_ & modeType::ATOMIC); }
+
 
     // Edit
 
-        //- Return managed pointer and release ownership
+        //- Return managed pointer and release ownership.
+        //  Likely invalidates is_append(), is_atomic().
         std::ostream* release() noexcept { return ptr_.release(); }
 
         //- Replace the managed pointer
+        //  Likely invalidates is_append(), is_atomic().
         void reset(std::ostream* ptr) noexcept { ptr_.reset(ptr); }
 
 
diff --git a/src/OpenFOAM/db/IOstreams/Fstreams/fstreamPointers.C b/src/OpenFOAM/db/IOstreams/Fstreams/fstreamPointers.C
index 01b9f460b65..e91aae193b9 100644
--- a/src/OpenFOAM/db/IOstreams/Fstreams/fstreamPointers.C
+++ b/src/OpenFOAM/db/IOstreams/Fstreams/fstreamPointers.C
@@ -88,14 +88,14 @@ Foam::ifstreamPointer::ifstreamPointer
 Foam::ofstreamPointer::ofstreamPointer() noexcept
 :
     ptr_(),
-    atomic_(false)
+    mode_(modeType::NONE)
 {}
 
 
 Foam::ofstreamPointer::ofstreamPointer(std::nullptr_t)
 :
     ptr_(new Foam::ocountstream),
-    atomic_(false)
+    mode_(modeType::NONE)
 {}
 
 
@@ -103,27 +103,33 @@ Foam::ofstreamPointer::ofstreamPointer
 (
     const fileName& pathname,
     IOstreamOption streamOpt,
-    const bool append,
-    const bool atomic
+    bool append,
+    bool atomic
 )
 :
     ptr_(),
-    atomic_(atomic)
+    mode_(modeType::NONE)
 {
-    std::ios_base::openmode mode
+    // Leave std::ios_base::trunc implicitly handled (easier for append mode)
+
+    std::ios_base::openmode openmode
     (
         std::ios_base::out | std::ios_base::binary
     );
 
     if (append)
     {
-        mode |= std::ios_base::app;
+        // Handle an "append-like" mode by opening "r+b" and NOT as "ab"
+        // - file already exists: Sets read position to start
+        // - file does not exist: Error
+
+        openmode |= std::ios_base::in;  // [SIC] - use read bit, not append!
 
         // Cannot append to gzstream
         streamOpt.compression(IOstreamOption::UNCOMPRESSED);
 
         // Cannot use append + atomic operation, without lots of extra work
-        atomic_ = false;
+        atomic = false;
     }
 
 
@@ -146,9 +152,9 @@ Foam::ofstreamPointer::ofstreamPointer
 
         #ifdef HAVE_LIBZ
         // TBD:
-        // atomic_ = true;  // Always treat COMPRESSED like an atomic
+        // atomic = true;  // Always treat COMPRESSED like an atomic
 
-        const fileName& target = (atomic_ ? pathname_tmp : pathname_gz);
+        const fileName& target = (atomic ? pathname_tmp : pathname_gz);
 
         // Remove old uncompressed version (if any)
         fType = Foam::type(pathname, false);
@@ -158,7 +164,7 @@ Foam::ofstreamPointer::ofstreamPointer
         }
 
         // Avoid writing into symlinked files (non-append mode)
-        if (!append || atomic_)
+        if (!append || atomic)
         {
             fType = Foam::type(target, false);
             if (fType == fileName::SYMLINK)
@@ -167,7 +173,7 @@ Foam::ofstreamPointer::ofstreamPointer
             }
         }
 
-        ptr_.reset(new ogzstream(target, mode));
+        ptr_.reset(new ogzstream(target, openmode));
 
         #else /* HAVE_LIBZ */
 
@@ -184,7 +190,7 @@ Foam::ofstreamPointer::ofstreamPointer
 
     if (IOstreamOption::COMPRESSED != streamOpt.compression())
     {
-        const fileName& target = (atomic_ ? pathname_tmp : pathname);
+        const fileName& target = (atomic ? pathname_tmp : pathname);
 
         // Remove old compressed version (if any)
         fType = Foam::type(pathname_gz, false);
@@ -194,7 +200,7 @@ Foam::ofstreamPointer::ofstreamPointer
         }
 
         // Avoid writing into symlinked files (non-append mode)
-        if (!append || atomic_)
+        if (!append || atomic)
         {
             fType = Foam::type(target, false);
             if (fType == fileName::SYMLINK)
@@ -203,7 +209,49 @@ Foam::ofstreamPointer::ofstreamPointer
             }
         }
 
-        ptr_.reset(new std::ofstream(target, mode));
+        // File pointer (std::ofstream)
+        auto filePtr = std::make_unique<std::ofstream>(target, openmode);
+
+        // Continue handling for pseudo-append mode (always non-atomic)
+        if (append)
+        {
+            if (filePtr->good())
+            {
+                // Success if file already exists.
+                // Set output position to the end - like std::ios_base::ate
+
+                filePtr->seekp(0, std::ios_base::end);
+            }
+            else
+            {
+                // Error if file does not already exist.
+                // Reopen as regular output mode only.
+
+                append = false;  // Did not open an existing file
+
+                if (filePtr->is_open())
+                {
+                    filePtr->close();
+                }
+                filePtr->clear();
+                filePtr->open
+                (
+                    target,
+                    (std::ios_base::out | std::ios_base::binary)
+                );
+            }
+        }
+        ptr_.reset(filePtr.release());
+    }
+
+    // Remember the settings actually used
+    if (append)
+    {
+        mode_ |= modeType::APPEND;
+    }
+    if (atomic)
+    {
+        mode_ |= modeType::ATOMIC;
     }
 }
 
@@ -212,8 +260,8 @@ Foam::ofstreamPointer::ofstreamPointer
 (
     const fileName& pathname,
     IOstreamOption::compressionType comp,
-    const bool append,
-    const bool atomic
+    bool append,
+    bool atomic
 )
 :
     ofstreamPointer
@@ -237,12 +285,12 @@ void Foam::ifstreamPointer::open
     // Forcibly close old stream (if any)
     ptr_.reset(nullptr);
 
-    const std::ios_base::openmode mode
+    const std::ios_base::openmode openmode
     (
         std::ios_base::in | std::ios_base::binary
     );
 
-    ptr_.reset(new std::ifstream(pathname, mode));
+    ptr_.reset(new std::ifstream(pathname, openmode));
 
     if (!ptr_->good())
     {
@@ -254,7 +302,7 @@ void Foam::ifstreamPointer::open
         {
             #ifdef HAVE_LIBZ
 
-            ptr_.reset(new igzstream(pathname_gz, mode));
+            ptr_.reset(new igzstream(pathname_gz, openmode));
 
             #else /* HAVE_LIBZ */
 
@@ -311,7 +359,7 @@ void Foam::ofstreamPointer::reopen(const std::string& pathname)
         gz->close();
         gz->clear();
 
-        if (atomic_)
+        if (mode_ & modeType::ATOMIC)
         {
             gz->open
             (
@@ -341,10 +389,12 @@ void Foam::ofstreamPointer::reopen(const std::string& pathname)
         }
         file->clear();
 
-        // Don't need original request to append since rewind implies
-        // trashing that anyhow.
+        // Discard opened in append-like mode and file already existed
+        // information since rewind implies trashing that anyhow.
+
+        mode_ &= ~modeType::APPEND;
 
-        if (atomic_)
+        if (mode_ & modeType::ATOMIC)
         {
             file->open
             (
@@ -367,7 +417,10 @@ void Foam::ofstreamPointer::reopen(const std::string& pathname)
 
 void Foam::ofstreamPointer::close(const std::string& pathname)
 {
-    if (!atomic_ || pathname.empty()) return;
+    if (pathname.empty() || !(mode_ & modeType::ATOMIC))
+    {
+        return;
+    }
 
     #ifdef HAVE_LIBZ
     auto* gz = dynamic_cast<ogzstream*>(ptr_.get());
-- 
GitLab