[PATCH] D50127: [Support] use larger character set for creating unique filenames

[Bob Haarman via Phabricator via llvm-commits]

inglorion updated this revision to Diff 158614.
inglorion added a comment.

Use 36-character set instead of 32-character set, per @zturner's suggestion.

I also realized that this would make the TempFileCollisions test flaky
(about 2% failure rate), so I modified it to get the expected failure
rate down below 1 per million.


https://reviews.llvm.org/D50127

Files:
  clang/test/Analysis/diagnostics/plist-multi-file.c
  llvm/lib/Support/Path.cpp
  llvm/unittests/Support/Path.cpp


Index: llvm/unittests/Support/Path.cpp
===================================================================
--- llvm/unittests/Support/Path.cpp
+++ llvm/unittests/Support/Path.cpp
@@ -692,7 +692,16 @@
   std::vector<fs::TempFile> TempFiles;
 
   auto TryCreateTempFile = [&]() {
+    // For the last available name, there is a 35/36 probability that we will
+    // pick a name that is already in use. TempFile::create() tries a number
+    // of names before giving up, but with only one character, this still leaves
+    // a chance of failure. We retry a few times here to get the probability
+    // of failure down to less than one in a million.
     Expected<fs::TempFile> T = fs::TempFile::create(Model);
+    for (int retries = 4; !T && retries > 0; --retries) {
+      consumeError(T.takeError());
+      T = fs::TempFile::create(Model);
+    }
     if (T) {
       TempFiles.push_back(std::move(*T));
       return true;
@@ -703,8 +712,8 @@
     }
   };
 
-  // We should be able to create exactly 16 temporary files.
-  for (int i = 0; i < 16; ++i)
+  // We should be able to create exactly 36 temporary files.
+  for (int i = 0; i < 36; ++i)
     EXPECT_TRUE(TryCreateTempFile());
   EXPECT_FALSE(TryCreateTempFile());
 
Index: llvm/lib/Support/Path.cpp
===================================================================
--- llvm/lib/Support/Path.cpp
+++ llvm/lib/Support/Path.cpp
@@ -197,8 +197,8 @@
     // Replace '%' with random chars.
     for (unsigned i = 0, e = ModelStorage.size(); i != e; ++i) {
       if (ModelStorage[i] == '%')
-        ResultPath[i] =
-            "0123456789abcdef"[sys::Process::GetRandomNumber() & 15];
+        ResultPath[i] = "0123456789abcdefghijklmnopqrstuvwxyz"
+            [sys::Process::GetRandomNumber() % 36];
     }
 
     // Try to open + create the file.
Index: clang/test/Analysis/diagnostics/plist-multi-file.c
===================================================================
--- clang/test/Analysis/diagnostics/plist-multi-file.c
+++ clang/test/Analysis/diagnostics/plist-multi-file.c
@@ -199,7 +199,7 @@
 // CHECK-NEXT:  </dict>
 // CHECK-NEXT:  <key>HTMLDiagnostics_files</key>
 // CHECK-NEXT:  <array>
-// CHECK-NEXT:   <string>report-{{([0-9a-f]{6})}}.html</string>
+// CHECK-NEXT:   <string>report-{{([0-9a-z]{6})}}.html</string>
 // CHECK-NEXT:  </array>
 // CHECK-NEXT:  </dict>
 // CHECK-NEXT: </array>


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D50127.158614.patch
Type: text/x-patch
Size: 2376 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180801/af0a92ce/attachment.bin>