Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(2889)

Unified Diff: base/files/file_util_win.cc

Issue 2788483005: Use GUID to generate unique temp file names and retire GetTempFileName (Closed)
Patch Set: Created 3 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: base/files/file_util_win.cc
diff --git a/base/files/file_util_win.cc b/base/files/file_util_win.cc
index 65dc5ce1e2fe7495624cc6d8c16b954395eb11ac..3be02b54368fb1faf6f6154408ef3af0fca2cafe 100644
--- a/base/files/file_util_win.cc
+++ b/base/files/file_util_win.cc
@@ -40,6 +40,34 @@ namespace {
const DWORD kFileShareAll =
FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE;
+// An array consists of digits and alphabet. It is used to generate random
+// prefix of a temp file name which is passed to GetTempFileName Windows API.
+const char kAlphaNumberCharSet[] =
gab 2017/04/03 16:40:49 I think you'll need to use FILE_PATH_LITERAL to ge
gab 2017/04/03 16:40:49 constexpr
chengx 2017/04/03 18:35:10 Done.
chengx 2017/04/03 18:35:11 Done.
+ "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz";
gab 2017/04/03 16:40:49 Filenames on Windows are case insensitive so don't
chengx 2017/04/03 18:35:10 Done.
+
+// The length of the temp file name prefix, which is used by GetTempFileName
+// Windows API.
+// GetTempFileName uses up to the first three characters of the input
+// prefix string as the prefix of the file name. Therefore, this constant is
+// set to three.
+// https://msdn.microsoft.com/en-us/library/windows/desktop/aa364991(v=vs.85).aspx
+const int kPrefixLength = 3;
gab 2017/04/03 16:40:50 constexpr
chengx 2017/04/03 18:35:11 Done.
+
+// Get a random prefix with length kPrefixLength of the temp file name.
+// The characters are obtained from kAlphaNumberCharSet.
+FilePath::StringType GetRandomPrefix() {
+ FilePath::StringType temp_file_name_prefix;
+ temp_file_name_prefix.resize(kPrefixLength);
+
+ size_t char_set_length = strlen(kAlphaNumberCharSet);
gab 2017/04/03 16:40:50 inline arraysize(kAlphaNumberCharSet) instead (it'
chengx 2017/04/03 18:35:10 Done.
+
+ srand(time(NULL));
+ for (size_t i = 0; i < kPrefixLength; i++)
+ temp_file_name_prefix[i] = kAlphaNumberCharSet[rand() % char_set_length];
gab 2017/04/03 16:40:49 Use base::RandGenerator(arraysize(kAlphaNumberChar
chengx 2017/04/03 18:35:10 Yes, no need to seed here if using base::RandGener
+
+ return temp_file_name_prefix;
+}
+
// Deletes all files and directories in a path.
// Returns false on the first failure it encounters.
bool DeleteFileRecursive(const FilePath& path,
@@ -342,7 +370,13 @@ bool CreateTemporaryFileInDir(const FilePath& dir, FilePath* temp_file) {
wchar_t temp_name[MAX_PATH + 1];
- if (!GetTempFileName(dir.value().c_str(), L"", 0, temp_name)) {
+ // Due to the algorithm used to generate file names, GetTempFileName Windows
+ // API can perform poorly when creating a large number of files with the same
+ // prefix. Therefore, making the prefix random should boost its performance.
+ // https://msdn.microsoft.com/en-us/library/windows/desktop/aa364991(v=vs.85).aspx
+
+ if (!GetTempFileName(dir.value().c_str(), GetRandomPrefix().c_str(), 0,
+ temp_name)) {
DPLOG(WARNING) << "Failed to get temporary file name in "
<< UTF16ToUTF8(dir.value());
return false;
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698