[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