[go: nahoru, domu]

Use StringPiece for the optional suffix in base::GetUniquePathNumber.

This removes the need to create/destroy a string when calling. This
change also adds a default value of an empty StringPiece to the suffix
so that callers who don't care about a suffix don't need to bother with
it. Finally, this change adds a unit test for the function, which it
apparently never had.

BUG=none
TBR=vasilii@chromium.org,vakh@chromium.org,eladalon@chromium.org,jamiewalch@chromium.org,carlosk@chromium.org

Change-Id: Iaf98d5237ffcc211bf9dfc2242742508f35bafac
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1752302
Commit-Queue: Greg Thompson <grt@chromium.org>
Auto-Submit: Greg Thompson <grt@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#686754}
diff --git a/base/files/file_util.cc b/base/files/file_util.cc
index e6e84a2..f11f456 100644
--- a/base/files/file_util.cc
+++ b/base/files/file_util.cc
@@ -281,20 +281,34 @@
 }
 
 int GetUniquePathNumber(const FilePath& path,
-                        const FilePath::StringType& suffix) {
-  bool have_suffix = !suffix.empty();
-  if (!PathExists(path) &&
-      (!have_suffix || !PathExists(FilePath(path.value() + suffix)))) {
-    return 0;
-  }
+                        FilePath::StringPieceType suffix) {
+  // Storage for use by is_unique to reduce heap churn when looping.
+  FilePath::StringType path_with_suffix;
 
-  FilePath new_path;
-  for (int count = 1; count <= kMaxUniqueFiles; ++count) {
-    new_path = path.InsertBeforeExtensionASCII(StringPrintf(" (%d)", count));
-    if (!PathExists(new_path) &&
-        (!have_suffix || !PathExists(FilePath(new_path.value() + suffix)))) {
-      return count;
+  // A function that returns true if |candidate| is unique with and without an
+  // optional suffix.
+  const auto is_unique = [suffix,
+                          &path_with_suffix](const base::FilePath& candidate) {
+    if (!PathExists(candidate)) {
+      if (suffix.empty())
+        return true;
+      path_with_suffix = candidate.value();
+      suffix.AppendToString(&path_with_suffix);
+      if (!PathExists(FilePath(path_with_suffix)))
+        return true;
     }
+    return false;
+  };
+
+  if (is_unique(path))
+    return 0;
+
+  std::string number;
+  for (int count = 1; count <= kMaxUniqueFiles; ++count) {
+    StringAppendF(&number, " (%d)", count);
+    if (is_unique(path.InsertBeforeExtensionASCII(number)))
+      return count;
+    number.clear();
   }
 
   return -1;
@@ -302,7 +316,7 @@
 
 FilePath GetUniquePath(const FilePath& path) {
   FilePath unique_path = path;
-  int uniquifier = GetUniquePathNumber(path, FilePath::StringType());
+  int uniquifier = GetUniquePathNumber(path);
   if (uniquifier > 0) {
     unique_path = unique_path.InsertBeforeExtensionASCII(
         StringPrintf(" (%d)", uniquifier));