[Lldb-commits] [PATCH] Fix warning about the use of mktemp and make platform agnostic by adding and using PipeBase::CreateWithUniqueName.

Zachary Turner zturner at google.com
Mon Feb 2 09:38:18 PST 2015


REPOSITORY
  rL LLVM

================
Comment at: include/lldb/Host/posix/PipePosix.h:83
@@ -75,2 +82,3 @@
     int m_fds[2];
+    llvm::SmallString<PATH_MAX> m_name;
 };
----------------
I think this would be better as an std::string.  The reason being that this means PipePosix is now going to take a huge amount of stack space, and it's not uncommon for pipes to be allocated on the stack.  Since the Pipe isn't used in any performance intensive code anyway, the extra heap allocation isn't really a big deal and it reduces the size of this class by a lot.

================
Comment at: source/Host/windows/PipeWindows.cpp:97-107
@@ +96,13 @@
+{
+    llvm::SmallString<128> name;
+    Error error;
+    do {
+        name = prefix;
+        name += "-";
+        for (unsigned i = 0; i < 6; ++i) {
+            name += "0123456789abcdef"[llvm::sys::Process::GetRandomNumber() & 15];
+        }
+        Error error = CreateNew(name, child_process_inherit);
+    } while (error.GetError() == ERROR_ALREADY_EXISTS);
+    return error;
+}
----------------
I would rather use UuidCreate [https://msdn.microsoft.com/en-us/library/windows/desktop/aa379205%28v=vs.85%29.aspx] and then UuidToString [https://msdn.microsoft.com/en-us/library/windows/desktop/aa379352(v=vs.85).aspx].  But if this compiles, and you don't feel comfortable / aren't able to make this change and test it on Windows, I can probably do that.

================
Comment at: source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp:785
@@ +784,3 @@
+                    return error;
+                strncpy(named_pipe_path, port_named_pipe.GetName().str().c_str(), PATH_MAX);
+                debugserver_args.AppendArgument("--named-pipe");
----------------
Should this be strncpy?  I feel like we should just be appending the argument directly, regardless of how long it is.  So nuke this whole named_pipe_path variable, and instead just call AppendArgument(port_named_pipe.GetName())

http://reviews.llvm.org/D7348

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the lldb-commits mailing list