fix(SubProcess): fix pipe fs close, add tests.

NEED TEST: BufferStream, AutoReadStream, SubProcess.

6 files changed, 87 insertions, 87 deletions

diff --git a/src/common/Format.cpp b/src/common/Format.cpp
index 0ae744fc..d58c90b7 100644
--- a/src/common/Format.cpp
+++ b/src/common/Format.cpp
@@ -1,25 +1,6 @@
 #include "cru/common/Format.h"
-#include <double-conversion/utils.h>
 
 namespace cru {
-
-double_conversion::DoubleToStringConverter kDefaultDoubleToStringConverter(
-    0, "infinity", "nan", 'e', -6, 21, 6, 1);
-
-String ToString(float value, StringView option) {
-  char buffer[50];
-  double_conversion::StringBuilder string_builder(buffer, sizeof(buffer));
-  kDefaultDoubleToStringConverter.ToShortestSingle(value, &string_builder);
-  return String::FromUtf8(std::string_view(buffer, string_builder.position()));
-}
-
-String ToString(double value, StringView option) {
-  char buffer[50];
-  double_conversion::StringBuilder string_builder(buffer, sizeof(buffer));
-  kDefaultDoubleToStringConverter.ToShortestSingle(value, &string_builder);
-  return String::FromUtf8(std::string_view(buffer, string_builder.position()));
-}
-
 namespace details {
 FormatToken ParsePlaceHolder(String place_holder_string) {
   if (place_holder_string.empty()) {
diff --git a/src/common/String.cpp b/src/common/String.cpp
index ddfa03a9..27712f01 100644
--- a/src/common/String.cpp
+++ b/src/common/String.cpp
@@ -33,7 +33,8 @@ String String::FromUtf8(const char* str, Index size) {
   Utf8CodePointIterator iter(str, size);
   for (auto cp : iter) {
     Utf16EncodeCodePointAppend(
-        cp, std::bind(&String::push_back, std::ref(result), std::placeholders::_1));
+        cp,
+        std::bind(&String::push_back, std::ref(result), std::placeholders::_1));
   }
   return result;
 }
@@ -330,23 +331,23 @@ Range String::RangeFromCodePointToCodeUnit(Range code_point_range) const {
   return View().RangeFromCodePointToCodeUnit(code_point_range);
 }
 
-int String::ParseToInt(Index* processed_characters_count, unsigned flags,
-                       int base) const {
+int String::ParseToInt(Index* processed_characters_count,
+                       StringToNumberFlag flags, int base) const {
   return View().ParseToInt(processed_characters_count, flags, base);
 }
 
 long long String::ParseToLongLong(Index* processed_characters_count,
-                                  unsigned flags, int base) const {
+                                  StringToNumberFlag flags, int base) const {
   return View().ParseToLongLong(processed_characters_count, flags, base);
 }
 
 float String::ParseToFloat(Index* processed_characters_count,
-                           unsigned flags) const {
+                           StringToNumberFlag flags) const {
   return View().ParseToFloat(processed_characters_count, flags);
 }
 
 double String::ParseToDouble(Index* processed_characters_count,
-                             unsigned flags) const {
+                             StringToNumberFlag flags) const {
   return View().ParseToDouble(processed_characters_count, flags);
 }
 
@@ -358,6 +359,11 @@ std::vector<double> String::ParseToDoubleList(value_type separator) const {
   return View().ParseToDoubleList(separator);
 }
 
+std::ostream& operator<<(std::ostream& os, const String& value) {
+  os << value.ToUtf8();
+  return os;
+}
+
 namespace {
 inline int Compare(char16_t left, char16_t right) {
   if (left < right) return -1;
@@ -540,7 +546,8 @@ std::string StringView::ToUtf8() const {
   std::string result;
   for (auto cp : CodePointIterator()) {
     Utf8EncodeCodePointAppend(
-        cp, std::bind(&std::string::push_back, std::ref(result), std::placeholders::_1));
+        cp, std::bind(&std::string::push_back, std::ref(result),
+                      std::placeholders::_1));
   }
   return result;
 }
@@ -563,17 +570,18 @@ Buffer StringView::ToUtf8Buffer(bool end_zero) const {
   return buffer;
 }
 
-int StringView::ParseToInt(Index* processed_characters_count, unsigned flags,
-                           int base) const {
+int StringView::ParseToInt(Index* processed_characters_count,
+                           StringToNumberFlag flags, int base) const {
   return ParseToInteger<int>(processed_characters_count, flags, base);
 }
 
 long long StringView::ParseToLongLong(Index* processed_characters_count,
-                                      unsigned flags, int base) const {
+                                      StringToNumberFlag flags,
+                                      int base) const {
   return ParseToInteger<long long>(processed_characters_count, flags, base);
 }
 
-static int MapStringToDoubleFlags(int flags) {
+static int MapStringToDoubleFlags(StringToNumberFlag flags) {
   int f = double_conversion::StringToDoubleConverter::ALLOW_CASE_INSENSIBILITY;
   if (flags & StringToNumberFlags::kAllowLeadingSpaces) {
     f |= double_conversion::StringToDoubleConverter::ALLOW_LEADING_SPACES;
@@ -588,12 +596,12 @@ static int MapStringToDoubleFlags(int flags) {
 }
 
 static double_conversion::StringToDoubleConverter CreateStringToDoubleConverter(
-    int flags) {
+    StringToNumberFlag flags) {
   return {MapStringToDoubleFlags(flags), 0.0, NAN, "inf", "nan"};
 }
 
 float StringView::ParseToFloat(Index* processed_characters_count,
-                               unsigned flags) const {
+                               StringToNumberFlag flags) const {
   int pcc;
   auto result = CreateStringToDoubleConverter(flags).StringToFloat(
       reinterpret_cast<const uc16*>(ptr_), static_cast<int>(size_), &pcc);
@@ -609,7 +617,7 @@ float StringView::ParseToFloat(Index* processed_characters_count,
 }
 
 double StringView::ParseToDouble(Index* processed_characters_count,
-                                 unsigned flags) const {
+                                 StringToNumberFlag flags) const {
   int pcc;
   auto result = CreateStringToDoubleConverter(flags).StringToDouble(
       reinterpret_cast<const uc16*>(ptr_), static_cast<int>(size_), &pcc);
diff --git a/src/common/StringToNumberConverter.cpp b/src/common/StringToNumberConverter.cpp
index e20e436c..7a926d3d 100644
--- a/src/common/StringToNumberConverter.cpp
+++ b/src/common/StringToNumberConverter.cpp
@@ -2,7 +2,7 @@
 #include "cru/common/Exception.h"
 
 namespace cru {
-bool StringToIntegerConverterImpl::CheckParams() const {
+bool StringToIntegerConverter::CheckParams() const {
   return base == 0 || base >= 2 & base <= 36;
 }
 
@@ -10,24 +10,20 @@ static bool IsSpace(char c) {
   return c == ' ' || c == '\t' || c == '\n' || c == '\r';
 }
 
-namespace {
-template <typename T>
-StringToIntegerConverterImplResult GenericParseInteger(
-    const StringToIntegerConverterImpl* converter, const T* const str,
-    const Index size, Index* processed_characters_count) {
+StringToIntegerResult StringToIntegerConverter::Parse(
+    const char* const str, const Index size,
+    Index* processed_characters_count) const {
   if (str == nullptr) throw std::invalid_argument("Invalid str.");
   if (size < 0) throw std::invalid_argument("Invalid size.");
-  if (!converter->CheckParams())
-    throw std::invalid_argument("Invalid parsing flags.");
+  if (!CheckParams()) throw std::invalid_argument("Invalid parsing flags.");
 
-  const bool throw_on_error =
-      (converter->flags & StringToNumberFlags::kThrowOnError) != 0;
+  const bool throw_on_error = flags.Has(StringToNumberFlags::kThrowOnError);
 
   auto const end = str + size;
 
   auto current = str;
 
-  if (converter->flags & StringToNumberFlags::kAllowLeadingSpaces) {
+  if (flags & StringToNumberFlags::kAllowLeadingSpaces) {
     while (current != end && IsSpace(*current)) {
       current++;
     }
@@ -64,9 +60,9 @@ StringToIntegerConverterImplResult GenericParseInteger(
     }
   }
 
-  int base = converter->base;
+  int real_base = base;
 
-  if (base == 0) {
+  if (real_base == 0) {
     if (*current == '0') {
       ++current;
       if (current == end) {
@@ -76,15 +72,15 @@ StringToIntegerConverterImplResult GenericParseInteger(
         return {negate, 0};
       } else if (*current == 'x' || *current == 'X') {
         ++current;
-        base = 16;
+        real_base = 16;
       } else if (*current == 'b' || *current == 'B') {
         ++current;
-        base = 2;
+        real_base = 2;
       } else {
-        base = 8;
+        real_base = 8;
       }
     } else {
-      base = 10;
+      real_base = 10;
     }
   }
 
@@ -100,7 +96,7 @@ StringToIntegerConverterImplResult GenericParseInteger(
   }
 
   const bool allow_leading_zero =
-      converter->flags & StringToNumberFlags::kAllowLeadingZeroForInteger;
+      flags.Has(StringToNumberFlags::kAllowLeadingZeroForInteger);
 
   while (current != end && *current == '0') {
     current++;
@@ -114,21 +110,22 @@ StringToIntegerConverterImplResult GenericParseInteger(
   }
 
   const bool allow_trailing_junk =
-      converter->flags & StringToNumberFlags::kAllowTrailingJunk;
+      flags.Has(StringToNumberFlags::kAllowTrailingJunk);
   const bool allow_trailing_spaces =
-      converter->flags & StringToNumberFlags::kAllowTrailingSpaces;
+      flags.Has(StringToNumberFlags::kAllowTrailingSpaces);
+
   unsigned long long result = 0;
 
   while (current != end) {
     const char c = *current;
-    if (c >= '0' && c <= (base > 10 ? '9' : base + '0' - 1)) {
-      result = result * base + c - '0';
+    if (c >= '0' && c <= (real_base > 10 ? '9' : real_base + '0' - 1)) {
+      result = result * real_base + c - '0';
       current++;
-    } else if (base > 10 && c >= 'a' && c <= (base + 'a' - 10 - 1)) {
-      result = result * base + c - 'a' + 10;
+    } else if (real_base > 10 && c >= 'a' && c <= (real_base + 'a' - 10 - 1)) {
+      result = result * real_base + c - 'a' + 10;
       current++;
-    } else if (base > 10 && c >= 'A' && c <= (base + 'A' - 10 - 1)) {
-      result = result * base + c - 'A' + 10;
+    } else if (real_base > 10 && c >= 'A' && c <= (real_base + 'A' - 10 - 1)) {
+      result = result * real_base + c - 'A' + 10;
       current++;
     } else if (allow_trailing_junk) {
       break;
@@ -169,17 +166,5 @@ StringToIntegerConverterImplResult GenericParseInteger(
 
   return {negate, result};
 }
-}  // namespace
 
-StringToIntegerConverterImplResult StringToIntegerConverterImpl::Parse(
-    const char* const str, const Index size,
-    Index* processed_characters_count) const {
-  return GenericParseInteger(this, str, size, processed_characters_count);
-}
-
-StringToIntegerConverterImplResult StringToIntegerConverterImpl::Parse(
-    const char16_t* const str, const Index size,
-    Index* processed_characters_count) const {
-  return GenericParseInteger(this, str, size, processed_characters_count);
-}
 }  // namespace cru
diff --git a/src/common/io/MemoryStream.cpp b/src/common/io/MemoryStream.cpp
index 7507875b..34116193 100644
--- a/src/common/io/MemoryStream.cpp
+++ b/src/common/io/MemoryStream.cpp
@@ -1,5 +1,7 @@
 #include "cru/common/io/MemoryStream.h"
 
+#include <cstring>
+
 namespace cru::io {
 MemoryStream::~MemoryStream() {
   if (release_func_) {
diff --git a/src/common/platform/unix/PosixSpawnSubProcess.cpp b/src/common/platform/unix/PosixSpawnSubProcess.cpp
index 8b521a5e..b5d68845 100644
--- a/src/common/platform/unix/PosixSpawnSubProcess.cpp
+++ b/src/common/platform/unix/PosixSpawnSubProcess.cpp
@@ -17,15 +17,15 @@ namespace cru::platform::unix {
 PosixSpawnSubProcessImpl::PosixSpawnSubProcessImpl()
     : pid_(0),
       exit_code_(0),
-      stdin_pipe_(UnixPipe::Usage::Send),
-      stdout_pipe_(UnixPipe::Usage::Receive),
-      stderr_pipe_(UnixPipe::Usage::Receive) {
+      stdin_pipe_(UnixPipe::Usage::Send, false),
+      stdout_pipe_(UnixPipe::Usage::Receive, false),
+      stderr_pipe_(UnixPipe::Usage::Receive, false) {
   stdin_stream_ = std::make_unique<UnixFileStream>(
-      stdin_pipe_.GetSelfFileDescriptor(), false, false, true, false);
+      stdin_pipe_.GetSelfFileDescriptor(), false, false, true, true);
   stdout_stream_ = std::make_unique<UnixFileStream>(
-      stdout_pipe_.GetSelfFileDescriptor(), false, true, false, false);
+      stdout_pipe_.GetSelfFileDescriptor(), false, true, false, true);
   stderr_stream_ = std::make_unique<UnixFileStream>(
-      stderr_pipe_.GetSelfFileDescriptor(), false, true, false, false);
+      stderr_pipe_.GetSelfFileDescriptor(), false, true, false, true);
 
   stdout_buffer_stream_ =
       std::make_unique<io::AutoReadStream>(stdout_stream_.get(), false);
@@ -95,15 +95,35 @@ void PosixSpawnSubProcessImpl::PlatformCreateProcess(
   error = posix_spawn_file_actions_adddup2(
       &file_actions, stderr_pipe_.GetOtherFileDescriptor(), STDERR_FILENO);
   check_error(u"Failed to call posix_spawn_file_actions_adddup2 on stderr.");
+
+  error = posix_spawn_file_actions_addclose(
+      &file_actions, stdin_pipe_.GetOtherFileDescriptor());
+  check_error(
+      u"Failed to call posix_spawn_file_actions_addclose on self fd of stdin.");
+  error = posix_spawn_file_actions_addclose(
+      &file_actions, stdout_pipe_.GetOtherFileDescriptor());
+  check_error(
+      u"Failed to call posix_spawn_file_actions_addclose on self fd stdout.");
+  error = posix_spawn_file_actions_addclose(
+      &file_actions, stderr_pipe_.GetOtherFileDescriptor());
+  check_error(
+      u"Failed to call posix_spawn_file_actions_addclose on self fd stderr.");
+
   error = posix_spawn_file_actions_addclose(
       &file_actions, stdin_pipe_.GetSelfFileDescriptor());
-  check_error(u"Failed to call posix_spawn_file_actions_addclose on stdin.");
+  check_error(
+      u"Failed to call posix_spawn_file_actions_addclose on parent fd of "
+      u"stdin.");
   error = posix_spawn_file_actions_addclose(
       &file_actions, stdout_pipe_.GetSelfFileDescriptor());
-  check_error(u"Failed to call posix_spawn_file_actions_addclose on stdout.");
+  check_error(
+      u"Failed to call posix_spawn_file_actions_addclose on parent fd of "
+      u"stdout.");
   error = posix_spawn_file_actions_addclose(
       &file_actions, stderr_pipe_.GetSelfFileDescriptor());
-  check_error(u"Failed to call posix_spawn_file_actions_addclose on stderr.");
+  check_error(
+      u"Failed to call posix_spawn_file_actions_addclose on parent fd of "
+      u"stderr.");
 
   posix_spawnattr_t attr;
   error = posix_spawnattr_init(&attr);
@@ -130,11 +150,11 @@ void PosixSpawnSubProcessImpl::PlatformCreateProcess(
   check_error(u"Failed to call posix_spawnp.");
 
   error = ::close(stdin_pipe_.GetOtherFileDescriptor());
-  check_error(u"Failed to close stdin.");
+  check_error(u"Failed to close child stdin.");
   error = ::close(stdout_pipe_.GetOtherFileDescriptor());
-  check_error(u"Failed to close stdout.");
+  check_error(u"Failed to close child stdout.");
   error = ::close(stderr_pipe_.GetOtherFileDescriptor());
-  check_error(u"Failed to close stderr.");
+  check_error(u"Failed to close child stderr.");
 }
 
 SubProcessExitResult PosixSpawnSubProcessImpl::PlatformWaitForProcess() {
diff --git a/src/common/platform/unix/UnixPipe.cpp b/src/common/platform/unix/UnixPipe.cpp
index 4d081d5e..f30c599e 100644
--- a/src/common/platform/unix/UnixPipe.cpp
+++ b/src/common/platform/unix/UnixPipe.cpp
@@ -7,8 +7,8 @@
 #include <unistd.h>
 
 namespace cru::platform::unix {
-UnixPipe::UnixPipe(Usage usage, UnixPipeFlag flags)
-    : usage_(usage), flags_(flags) {
+UnixPipe::UnixPipe(Usage usage, bool auto_close, UnixPipeFlag flags)
+    : usage_(usage), auto_close_(auto_close), flags_(flags) {
   int fds[2];
   if (pipe(fds) != 0) {
     throw ErrnoException(u"Failed to create unix pipe.");
@@ -40,8 +40,12 @@ int UnixPipe::GetOtherFileDescriptor() {
 }
 
 UnixPipe::~UnixPipe() {
-  if (close(GetSelfFileDescriptor()) != 0) {
-    CRU_LOG_ERROR(u"Failed to close unix pipe file descriptor.");
+  if (auto_close_) {
+    auto self_fd = GetSelfFileDescriptor();
+    if (::close(self_fd) != 0) {
+      CRU_LOG_ERROR(u"Failed to close unix pipe file descriptor {}, errno {}.",
+                    self_fd, errno);
+    }
   }
 }
 }  // namespace cru::platform::unix