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));