From 206da68ef3b725bbc3a3b5555802f76f4ed0856d Mon Sep 17 00:00:00 2001
From: Yoonsoo Kim <yoonsoo.kim@mongodb.com>
Date: Fri, 6 Jan 2023 01:01:06 +0000
Subject: SERVER-72441 Log an error message for remove() only when it's
 necessary

---
 src/mongo/db/storage/named_pipe_posix.cpp | 45 +++++++++++++++++++++----------
 1 file changed, 31 insertions(+), 14 deletions(-)

diff --git a/src/mongo/db/storage/named_pipe_posix.cpp b/src/mongo/db/storage/named_pipe_posix.cpp
index 63d955a8df5..b5dfd5dd012 100644
--- a/src/mongo/db/storage/named_pipe_posix.cpp
+++ b/src/mongo/db/storage/named_pipe_posix.cpp
@@ -31,6 +31,7 @@
 
 #include "mongo/db/storage/named_pipe.h"
 
+#include <cerrno>
 #include <cstdio>
 #include <fmt/format.h>
 #include <string>
@@ -41,6 +42,7 @@
 #include "mongo/db/storage/io_error_message.h"
 #include "mongo/logv2/log.h"
 #include "mongo/stdx/thread.h"
+#include "mongo/util/assert_util.h"
 
 namespace mongo {
 using namespace fmt::literals;
@@ -48,14 +50,24 @@ using namespace fmt::literals;
 #define MONGO_LOGV2_DEFAULT_COMPONENT ::mongo::logv2::LogComponent::kTest
 
 namespace {
-// Removes the named pipe and logs an info message if there's an error. The info message should be
-// fine since this is a test-only implementation.
-void removeAndLog(const char* pipeAbsolutePath) {
-    // Suppress the expected non-error "error" ENOENT ("No such file or directory").
-    if ((remove(pipeAbsolutePath) < 0) && (errno != ENOENT)) {
-        LOGV2_INFO(7097000,
-                   "Failed to remove",
-                   "error"_attr = getErrorMessage("remove", pipeAbsolutePath));
+// 'rc' must be the return code from POSIX-like OS I/O APIs.
+inline bool hasSucceeded(int rc) {
+    // POSIX-like I/O APIs return 0 for the successful operation.
+    return rc == 0;
+}
+
+// Removes the named pipe and logs an error message when
+// - either 'ignoreNoEntError' == true and there's an error other than the ENOENT error
+// - or 'ignoreNoEntError' == false and there's any error
+void removeNamedPipe(bool ignoreNoEntError, const char* pipeAbsolutePath) {
+    if (!hasSucceeded(remove(pipeAbsolutePath))) {
+        if (ignoreNoEntError && errno == ENOENT) {
+            return;
+        }
+
+        LOGV2_ERROR(7097000,
+                    "Failed to remove",
+                    "error"_attr = getErrorMessage("remove", pipeAbsolutePath));
     }
 }
 }  // namespace
@@ -63,18 +75,18 @@ void removeAndLog(const char* pipeAbsolutePath) {
 NamedPipeOutput::NamedPipeOutput(const std::string& pipeDir, const std::string& pipeRelativePath)
     : _pipeAbsolutePath(pipeDir + pipeRelativePath), _ofs() {
     // Just in case that uncleaned-up named pipe is still there. This is a test-only implementation
-    // and so, it should be fine to just remove it.
-    removeAndLog(_pipeAbsolutePath.c_str());
+    // and so, it should be fine to just remove it and ignore the ENOENT error.
+    removeNamedPipe(true /*ignoreNoEntError*/, _pipeAbsolutePath.c_str());
     uassert(7005005,
             "Failed to create a named pipe, error: {}"_format(
                 getErrorMessage("mkfifo", _pipeAbsolutePath)),
-            mkfifo(_pipeAbsolutePath.c_str(), 0664) == 0);
+            hasSucceeded(mkfifo(_pipeAbsolutePath.c_str(), 0664)));
 }
 
 NamedPipeOutput::~NamedPipeOutput() {
     close();
     // Makes sure that the named pipe is removed.
-    removeAndLog(_pipeAbsolutePath.c_str());
+    removeNamedPipe(false /*ignoreNoEntError*/, _pipeAbsolutePath.c_str());
 }
 
 void NamedPipeOutput::open() {
@@ -133,16 +145,21 @@ void NamedPipeInput::doOpen() {
     // retry) in case the pipe writer has not finished creating the pipe yet.
     int retries = 0;
     int sleepMs = 1;
+    bool opened;
     do {
         _ifs.open(_pipeAbsolutePath.c_str(), std::ios::binary | std::ios::in);
-        if (!_ifs.is_open()) {
+        opened = _ifs.is_open();
+        if (!opened) {
+            uassert(ErrorCodes::FileNotOpen,
+                    "error = {}"_format(getErrorMessage("open", _pipeAbsolutePath)),
+                    errno == ENOENT);
             stdx::this_thread::sleep_for(stdx::chrono::milliseconds(sleepMs));
             ++retries;
             if (retries % 1000 == 0) {
                 sleepMs *= 2;
             }
         }
-    } while (!_ifs.is_open() && retries <= 5000);
+    } while (!opened && retries <= 5000);
     if (retries > 1000) {
         LOGV2_WARNING(7184900,
                       "NamedPipeInput::doOpen() waited for pipe longer than 1 sec",
-- 
cgit v1.2.1