[Lldb-commits] [PATCH] Use GUIDs to make pipe names unique on Windows.

Zachary Turner zturner at google.com
Mon Feb 9 10:32:23 PST 2015


General comment.  It's usually good to create diffs with -U999999.  This will include the entire file that changed in the diffs, as opposed to just changed lines.  Useful for generating more diff context so reviewer can see more of the file.  (Although in this case it's my fault for forgetting to tell you about that).


================
Comment at: source/Host/windows/PipeWindows.cpp:100-101
@@ -98,2 +99,4 @@
     Error error;
-    do {
+    ::UUID unique_id;
+    ::RPC_CSTR unique_string;
+    if (SUCCEEDED(::UuidCreate(&unique_id)) &&
----------------
Don't use global scope identifiers before typenames unless it's necessary to disambiguate.  The same rule doesn't apply to functions though, ::GlobalFunction() is ok.

================
Comment at: source/Host/windows/PipeWindows.cpp:102-103
@@ +101,4 @@
+    ::RPC_CSTR unique_string;
+    if (SUCCEEDED(::UuidCreate(&unique_id)) &&
+        SUCCEEDED(::UuidToStringA(&unique_id, &unique_string)))
+    {
----------------
Is it correct to use SUCCEEDED() / FAILED() with RPC calls?  They dont' return HRESULTs.  Seems like the return values are just standard windows error codes.

================
Comment at: source/Host/windows/PipeWindows.cpp:115
@@ +114,3 @@
+        // creation could have failed.
+        error.SetError(ERROR_NOT_ENOUGH_MEMORY, eErrorTypeWin32);
+    }
----------------
I would just set the error to the return code of whichever call failed.  Normally we use GetLastError() here, but The RPC functions seem to return the error value directly.

http://reviews.llvm.org/D7509

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






More information about the lldb-commits mailing list