Setting the size of the ipc pipe buffers is not mandatory, ref #614

epoupon · Feb 16, 2025 · 923c632 · 923c632
1 parent 768c7ed
commit 923c632
Show file tree

Hide file tree

Showing 4 changed files with 42 additions and 17 deletions.
diff --git a/src/libs/core/impl/ArchiveZipper.cpp b/src/libs/core/impl/ArchiveZipper.cpp
@@ -21,13 +21,13 @@
 
 #include <algorithm>
 #include <cassert>
-#include <cstring> // strerror
 #include <fstream>
 
 #include <archive.h>
 #include <archive_entry.h>
 
 #include "core/ILogger.hpp"
+#include "core/String.hpp"
 
 namespace lms::zip
 {
@@ -45,7 +45,7 @@ namespace lms::zip
         }
 
         FileException(const std::filesystem::path& p, std::string_view message, int err)
-            : Exception{ "File '" + p.string() + "': " + std::string{ message } + ": " + ::strerror(err) }
+            : Exception{ "File '" + p.string() + "': " + std::string{ message } + ": " + core::stringUtils::systemErrorToString(err) }
         {
         }
     };
@@ -75,7 +75,7 @@ namespace lms::zip
     {
         const int res{ ::archive_write_free(arch) };
         if (res != ARCHIVE_OK)
-            LMS_LOG(UTILS, ERROR, "Failure while freeing archive control struct: " << std::string{ ::strerror(res) });
+            LMS_LOG(UTILS, ERROR, "Failure while freeing archive control struct: error code = " << res);
     }
 
     void ArchiveZipper::ArchiveEntryDeleter::operator()(struct ::archive_entry* archEntry)

diff --git a/src/libs/core/impl/ChildProcess.cpp b/src/libs/core/impl/ChildProcess.cpp
@@ -20,10 +20,9 @@
 #include "ChildProcess.hpp"
 
 #include <cerrno>
-#include <cstring>
+#include <cstddef>
 #include <fcntl.h>
 #include <signal.h>
-#include <stdexcept>
 #include <sys/types.h>
 #include <sys/wait.h>
 #include <unistd.h>
@@ -35,8 +34,8 @@
 #include <boost/asio/buffer.hpp>
 #include <boost/asio/read.hpp>
 
-#include "core/Exception.hpp"
 #include "core/ILogger.hpp"
+#include "core/String.hpp"
 
 namespace lms::core
 {
@@ -46,7 +45,7 @@ namespace lms::core
         {
         public:
             SystemException(int err, const std::string& errMsg)
-                : ChildProcessException{ errMsg + ": " + ::strerror(err) }
+                : ChildProcessException{ errMsg + ": " + stringUtils::systemErrorToString(err) }
             {
             }
 
@@ -86,14 +85,30 @@ namespace lms::core
             throw SystemException{ errno, "fcntl failed to set FD_CLOEXEC!" };
 
 #if defined(__linux__) && defined(F_SETPIPE_SZ)
+        for (const int fd : { pipefd[0], pipefd[1] })
         {
-            // Just a hint here to prevent the writer from writing too many bytes ahead of the reader
-            constexpr std::size_t pipeSize{ 65536 * 4 };
-
-            if (fcntl(pipefd[0], F_SETPIPE_SZ, pipeSize) == -1)
-                throw SystemException{ errno, "fcntl failed!" };
-            if (fcntl(pipefd[1], F_SETPIPE_SZ, pipeSize) == -1)
-                throw SystemException{ errno, "fcntl failed!" };
+            constexpr std::size_t targetPipeSize{ static_cast<long>(65'536) * 4 };
+            std::size_t currentPipeSize{ 65'536 }; // common default value
+    #if defined(F_GETPIPE_SZ)
+            const int pipeSizeRes{ fcntl(fd, F_GETPIPE_SZ) };
+            if (pipeSizeRes == -1)
+            {
+                const int err{ errno };
+                LMS_LOG(CHILDPROCESS, DEBUG, "F_GETPIPE_SZ failed: " << stringUtils::systemErrorToString(err));
+            }
+            else
+            {
+                currentPipeSize = pipeSizeRes;
+            }
+    #endif
+            if (currentPipeSize < targetPipeSize)
+            {
+                if (fcntl(fd, F_SETPIPE_SZ, targetPipeSize) == -1)
+                {
+                    const int err{ errno };
+                    LMS_LOG(CHILDPROCESS, DEBUG, "F_SETPIPE_SZ failed: " << stringUtils::systemErrorToString(err));
+                }
+            }
         }
 #endif
 
@@ -153,7 +168,7 @@ namespace lms::core
         // process may already have finished
         LMS_LOG(CHILDPROCESS, DEBUG, "Killing child process...");
         if (::kill(_childPID, SIGKILL) == -1)
-            LMS_LOG(CHILDPROCESS, DEBUG, "Kill failed: " << ::strerror(errno));
+            LMS_LOG(CHILDPROCESS, DEBUG, "Kill failed: " << stringUtils::systemErrorToString(errno));
     }
 
     bool ChildProcess::wait(bool block)
@@ -165,7 +180,7 @@ namespace lms::core
 
         if (pid == -1)
             throw SystemException{ errno, "waitpid failed!" };
-        else if (pid == 0)
+        if (pid == 0)
             return false;
 
         if (WIFEXITED(wstatus))

diff --git a/src/libs/core/impl/String.cpp b/src/libs/core/impl/String.cpp
@@ -20,6 +20,8 @@
 #include "core/String.hpp"
 
 #include <algorithm>
+#include <array>
+#include <cstring>
 #include <iomanip>
 #include <utility>
 
@@ -504,4 +506,11 @@ namespace lms::core::stringUtils
 
         return "[" + std::to_string(mins) + ":" + (secs < 10 ? "0" : "") + std::to_string(secs) + "." + (millis < 100 ? (millis < 10 ? "00" : "0") : "") + std::to_string(millis) + "]";
     }
+
+    std::string systemErrorToString(int err)
+    {
+        std::array<char, 128> buffer{};
+        ::strerror_r(err, buffer.data(), buffer.size());
+        return std::string{ buffer.data() };
+    }
 } // namespace lms::core::stringUtils
diff --git a/src/libs/core/include/core/String.hpp b/src/libs/core/include/core/String.hpp
@@ -20,7 +20,6 @@
 #pragma once
 
 #include <chrono>
-#include <initializer_list>
 #include <optional>
 #include <span>
 #include <sstream>
@@ -120,4 +119,6 @@ namespace lms::core::stringUtils
 
     // to "[minutes:seconds.milliseconds]"
     std::string formatTimestamp(std::chrono::milliseconds timestamp);
+
+    std::string systemErrorToString(int err);
 } // namespace lms::core::stringUtils