Fix warning about the use of mktemp and make platform agnostic by adding and using PipeBase::CreateWithUniqueName - on behalf of flackr.

http://reviews.llvm.org/D7348

llvm-svn: 228307

6 files changed, 71 insertions, 25 deletions

diff --git a/lldb/include/lldb/Host/PipeBase.h b/lldb/include/lldb/Host/PipeBase.h
index 8cad2507d32..5ef2bb53028 100644
--- a/lldb/include/lldb/Host/PipeBase.h
+++ b/lldb/include/lldb/Host/PipeBase.h
@@ -14,6 +14,7 @@
 #include <string>
 
 #include "lldb/Core/Error.h"
+#include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 
 namespace lldb_private
@@ -25,6 +26,7 @@ class PipeBase
 
     virtual Error CreateNew(bool child_process_inherit) = 0;
     virtual Error CreateNew(llvm::StringRef name, bool child_process_inherit) = 0;
+    virtual Error CreateWithUniqueName(llvm::StringRef prefix, bool child_process_inherit, llvm::SmallVectorImpl<char>& name) = 0;
 
     virtual Error OpenAsReader(llvm::StringRef name, bool child_process_inherit) = 0;
 
diff --git a/lldb/include/lldb/Host/posix/PipePosix.h b/lldb/include/lldb/Host/posix/PipePosix.h
index 0ab3ff7f677..fbdac66149d 100644
--- a/lldb/include/lldb/Host/posix/PipePosix.h
+++ b/lldb/include/lldb/Host/posix/PipePosix.h
@@ -36,6 +36,8 @@ public:
     Error
     CreateNew(llvm::StringRef name, bool child_process_inherit) override;
     Error
+    CreateWithUniqueName(llvm::StringRef prefix, bool child_process_inherit, llvm::SmallVectorImpl<char>& name) override;
+    Error
     OpenAsReader(llvm::StringRef name, bool child_process_inherit) override;
     Error
     OpenAsWriterWithTimeout(llvm::StringRef name, bool child_process_inherit, const std::chrono::microseconds &timeout) override;
diff --git a/lldb/include/lldb/Host/windows/PipeWindows.h b/lldb/include/lldb/Host/windows/PipeWindows.h
index 02b7d470c49..00307ed49a7 100644
--- a/lldb/include/lldb/Host/windows/PipeWindows.h
+++ b/lldb/include/lldb/Host/windows/PipeWindows.h
@@ -31,6 +31,7 @@ class PipeWindows : public PipeBase
 
     Error CreateNew(bool child_process_inherit) override;
     Error CreateNew(llvm::StringRef name, bool child_process_inherit) override;
+    Error CreateWithUniqueName(llvm::StringRef prefix, bool child_process_inherit, llvm::SmallVectorImpl<char>& name) override;
     Error OpenAsReader(llvm::StringRef name, bool child_process_inherit) override;
     Error OpenAsWriterWithTimeout(llvm::StringRef name, bool child_process_inherit, const std::chrono::microseconds &timeout) override;
 
diff --git a/lldb/source/Host/posix/PipePosix.cpp b/lldb/source/Host/posix/PipePosix.cpp
index 02838ec5124..1650f1e7979 100644
--- a/lldb/source/Host/posix/PipePosix.cpp
+++ b/lldb/source/Host/posix/PipePosix.cpp
@@ -9,12 +9,16 @@
 
 #include "lldb/Host/posix/PipePosix.h"
 #include "lldb/Host/FileSystem.h"
+#include "lldb/Host/HostInfo.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/Support/FileSystem.h"
 
 #include <functional>
 #include <thread>
 
 #include <errno.h>
 #include <fcntl.h>
+#include <limits.h>
 #include <unistd.h>
 #include <sys/types.h>
 #include <sys/stat.h>
@@ -183,6 +187,37 @@ PipePosix::CreateNew(llvm::StringRef name, bool child_process_inherit)
 }
 
 Error
+PipePosix::CreateWithUniqueName(llvm::StringRef prefix, bool child_process_inherit, llvm::SmallVectorImpl<char>& name)
+{
+    llvm::SmallString<PATH_MAX> named_pipe_path;
+    llvm::SmallString<PATH_MAX> pipe_spec((prefix + ".%%%%%%").str());
+    FileSpec tmpdir_file_spec;
+    tmpdir_file_spec.Clear();
+    if (HostInfo::GetLLDBPath(ePathTypeLLDBTempSystemDir, tmpdir_file_spec))
+    {
+        tmpdir_file_spec.AppendPathComponent(pipe_spec.c_str());
+    }
+    else
+    {
+        tmpdir_file_spec.AppendPathComponent("/tmp");
+        tmpdir_file_spec.AppendPathComponent(pipe_spec.c_str());
+    }
+
+    // It's possible that another process creates the target path after we've
+    // verified it's available but before we create it, in which case we
+    // should try again.
+    Error error;
+    do {
+        llvm::sys::fs::createUniqueFile(tmpdir_file_spec.GetPath().c_str(), named_pipe_path);
+        error = CreateNew(named_pipe_path, child_process_inherit);
+    } while (error.GetError() == EEXIST);
+
+    if (error.Success())
+        name = named_pipe_path;
+    return error;
+}
+
+Error
 PipePosix::OpenAsReader(llvm::StringRef name, bool child_process_inherit)
 {
     if (CanRead() || CanWrite())
diff --git a/lldb/source/Host/windows/PipeWindows.cpp b/lldb/source/Host/windows/PipeWindows.cpp
index 852630eb04b..eccc73810f3 100644
--- a/lldb/source/Host/windows/PipeWindows.cpp
+++ b/lldb/source/Host/windows/PipeWindows.cpp
@@ -9,6 +9,8 @@
 
 #include "lldb/Host/windows/PipeWindows.h"
 
+#include "llvm/ADT/SmallString.h"
+#include "llvm/Support/Process.h"
 #include "llvm/Support/raw_ostream.h"
 
 #include <fcntl.h>
@@ -90,6 +92,24 @@ PipeWindows::CreateNew(llvm::StringRef name, bool child_process_inherit)
 }
 
 Error
+PipeWindows::CreateWithUniqueName(llvm::StringRef prefix, bool child_process_inherit, llvm::SmallVectorImpl<char>& name)
+{
+    llvm::SmallString<128> pipe_name;
+    Error error;
+    do {
+        pipe_name = prefix;
+        pipe_name += "-";
+        for (unsigned i = 0; i < 6; i++) {
+            pipe_name += "0123456789abcdef"[llvm::sys::Process::GetRandomNumber() & 15];
+        }
+        Error error = CreateNew(pipe_name, child_process_inherit);
+    } while (error.GetError() == ERROR_ALREADY_EXISTS);
+    if (error.Success())
+        name = pipe_name;
+    return error;
+}
+
+Error
 PipeWindows::OpenAsReader(llvm::StringRef name, bool child_process_inherit)
 {
     if (CanRead() || CanWrite())
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
index ef903fb6dd6..d633b3eaf34 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
@@ -30,6 +30,7 @@
 #include "lldb/Host/ThreadLauncher.h"
 #include "lldb/Host/TimeValue.h"
 #include "lldb/Target/Process.h"
+#include "llvm/ADT/SmallString.h"
 
 // Project includes
 #include "ProcessGDBRemoteLog.h"
@@ -764,8 +765,7 @@ GDBRemoteCommunication::StartDebugserverProcess (const char *hostname,
             debugserver_args.AppendArgument("--setsid");
         }
 
-        char named_pipe_path[PATH_MAX];
-        named_pipe_path[0] = '\0';
+        llvm::SmallString<PATH_MAX> named_pipe_path;
         Pipe port_named_pipe;
 
         bool listen = false;
@@ -779,25 +779,11 @@ GDBRemoteCommunication::StartDebugserverProcess (const char *hostname,
             {
                 // Binding to port zero, we need to figure out what port it ends up
                 // using using a named pipe...
-                FileSpec tmpdir_file_spec;
-                if (HostInfo::GetLLDBPath(ePathTypeLLDBTempSystemDir, tmpdir_file_spec))
-                {
-                    tmpdir_file_spec.AppendPathComponent("debugserver-named-pipe.XXXXXX");
-                    strncpy(named_pipe_path, tmpdir_file_spec.GetPath().c_str(), sizeof(named_pipe_path));
-                }
-                else
-                {
-                    strncpy(named_pipe_path, "/tmp/debugserver-named-pipe.XXXXXX", sizeof(named_pipe_path));
-                }
-
-                if (::mktemp (named_pipe_path))
-                {
-                    error = port_named_pipe.CreateNew(named_pipe_path, false);
-                    if (error.Fail())
-                        return error;
-                    debugserver_args.AppendArgument("--named-pipe");
-                    debugserver_args.AppendArgument(named_pipe_path);
-                }
+                error = port_named_pipe.CreateWithUniqueName("debugserver-named-pipe", false, named_pipe_path);
+                if (error.Fail())
+                    return error;
+                debugserver_args.AppendArgument("--named-pipe");
+                debugserver_args.AppendArgument(named_pipe_path.c_str());
             }
             else
             {
@@ -877,7 +863,7 @@ GDBRemoteCommunication::StartDebugserverProcess (const char *hostname,
         
         if (error.Success() && launch_info.GetProcessID() != LLDB_INVALID_PROCESS_ID)
         {
-            if (named_pipe_path[0])
+            if (named_pipe_path.size() > 0)
             {
                 error = port_named_pipe.OpenAsReader(named_pipe_path, false);
                 if (error.Success())
@@ -897,7 +883,7 @@ GDBRemoteCommunication::StartDebugserverProcess (const char *hostname,
                     else
                     {
                         if (log)
-                            log->Printf("GDBRemoteCommunication::%s() failed to read a port value from named pipe %s: %s", __FUNCTION__, named_pipe_path, error.AsCString());
+                            log->Printf("GDBRemoteCommunication::%s() failed to read a port value from named pipe %s: %s", __FUNCTION__, named_pipe_path.c_str(), error.AsCString());
 
                     }
                     port_named_pipe.Close();
@@ -905,13 +891,13 @@ GDBRemoteCommunication::StartDebugserverProcess (const char *hostname,
                 else
                 {
                     if (log)
-                        log->Printf("GDBRemoteCommunication::%s() failed to open named pipe %s for reading: %s", __FUNCTION__, named_pipe_path, error.AsCString());
+                        log->Printf("GDBRemoteCommunication::%s() failed to open named pipe %s for reading: %s", __FUNCTION__, named_pipe_path.c_str(), error.AsCString());
                 }
                 const auto err = port_named_pipe.Delete(named_pipe_path);
                 if (err.Fail())
                 {
                     if (log)
-                        log->Printf ("GDBRemoteCommunication::%s failed to delete pipe %s: %s", __FUNCTION__, named_pipe_path, err.AsCString());
+                        log->Printf ("GDBRemoteCommunication::%s failed to delete pipe %s: %s", __FUNCTION__, named_pipe_path.c_str(), err.AsCString());
                 }
             }
             else if (listen)