Modernize PDFium strings operator==().
Use the technique of friending the two-arg form to avoid having to
create reflective forms of the comparisons and the != operator.
-- Delegate to views when efficient to do so.
-- Avoid walking the entire string when comparing against a ptr.
-- Move TODO() to header.
Change-Id: I27c5e00354e77aa597d0614523a85477e5a1d75a
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/131290
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
diff --git a/core/fxcrt/bytestring.cpp b/core/fxcrt/bytestring.cpp
index 5dc5c8f..70eaf43 100644
--- a/core/fxcrt/bytestring.cpp
+++ b/core/fxcrt/bytestring.cpp
@@ -157,7 +157,6 @@
}
}
-// TODO(tsepez): Should be UNSAFE_BUFFER_USAGE.
ByteString& ByteString::operator=(const char* str) {
if (!str || !str[0]) {
clear();
@@ -224,51 +223,16 @@
return *this;
}
-// TODO(tsepez): Should be UNSAFE_BUFFER_USAGE
-bool ByteString::operator==(const char* ptr) const {
- if (!data_) {
- return !ptr || !ptr[0];
+bool operator==(const ByteString& lhs, const char* rhs) {
+ if (lhs.IsEmpty()) {
+ return !rhs || !rhs[0];
}
-
- if (!ptr) {
- return data_->data_length_ == 0;
- }
-
- // SAFETY: `data_length_` is within `string_`, and the strlen() call
- // (whose own safety is required from the caller) ensures there are
- // `data_length_` bytes at `ptr` before the terminator.
- return UNSAFE_BUFFERS(strlen(ptr)) == data_->data_length_ &&
- UNSAFE_BUFFERS(
- FXSYS_memcmp(ptr, data_->string_, data_->data_length_)) == 0;
-}
-
-bool ByteString::operator==(ByteStringView str) const {
- if (!data_) {
- return str.IsEmpty();
- }
-
- // SAFETY: `str` has `GetLength()` valid bytes in `unterminated_c_str()`,
- // `data_length_` is within `string_`, and equality comparison.
- return data_->data_length_ == str.GetLength() &&
- UNSAFE_BUFFERS(FXSYS_memcmp(data_->string_, str.unterminated_c_str(),
- str.GetLength())) == 0;
-}
-
-bool ByteString::operator==(const ByteString& other) const {
- if (data_ == other.data_) {
- return true;
- }
- if (IsEmpty()) {
- return other.IsEmpty();
- }
- if (other.IsEmpty()) {
+ if (!rhs) {
return false;
}
- // SAFETY: data_length_ describes the length of string_.
- return other.data_->data_length_ == data_->data_length_ &&
- UNSAFE_BUFFERS(memcmp(other.data_->string_, data_->string_,
- data_->data_length_)) == 0;
+ // SAFETY: required from caller.
+ return UNSAFE_BUFFERS(strcmp(lhs.data_->string_, rhs)) == 0;
}
// TODO(tsepez): Should be UNSAFE_BUFFER_USAGE.
diff --git a/core/fxcrt/bytestring.h b/core/fxcrt/bytestring.h
index 2ba59fc..3beb998 100644
--- a/core/fxcrt/bytestring.h
+++ b/core/fxcrt/bytestring.h
@@ -58,13 +58,14 @@
int Compare(ByteStringView str) const;
bool EqualNoCase(ByteStringView str) const;
- bool operator==(const char* ptr) const;
- bool operator==(ByteStringView str) const;
- bool operator==(const ByteString& other) const;
-
- bool operator!=(const char* ptr) const { return !(*this == ptr); }
- bool operator!=(ByteStringView str) const { return !(*this == str); }
- bool operator!=(const ByteString& other) const { return !(*this == other); }
+ // TODO(tsepez): The char* form should be UNSAFE_BUFFER_USAGE.
+ friend bool operator==(const ByteString& lhs, const char* rhs);
+ friend bool operator==(const ByteString& lhs, ByteStringView rhs) {
+ return lhs.AsStringView() == rhs;
+ }
+ friend bool operator==(const ByteString& lhs, const ByteString& rhs) {
+ return lhs.AsStringView() == rhs.AsStringView();
+ }
bool operator<(const char* ptr) const;
bool operator<(ByteStringView str) const;
@@ -106,18 +107,6 @@
friend class StringPool_ByteString_Test;
};
-inline bool operator==(const char* lhs, const ByteString& rhs) {
- return rhs == lhs;
-}
-inline bool operator==(ByteStringView lhs, const ByteString& rhs) {
- return rhs == lhs;
-}
-inline bool operator!=(const char* lhs, const ByteString& rhs) {
- return rhs != lhs;
-}
-inline bool operator!=(ByteStringView lhs, const ByteString& rhs) {
- return rhs != lhs;
-}
inline bool operator<(const char* lhs, const ByteString& rhs) {
return rhs.Compare(lhs) > 0;
}
diff --git a/core/fxcrt/widestring.cpp b/core/fxcrt/widestring.cpp
index 55fc79c..ad17637 100644
--- a/core/fxcrt/widestring.cpp
+++ b/core/fxcrt/widestring.cpp
@@ -525,52 +525,16 @@
return *this;
}
-// Should be UNSAFE_BUFFER_USAGE.
-bool WideString::operator==(const wchar_t* ptr) const {
- if (!data_) {
- return !ptr || !ptr[0];
+bool operator==(const WideString& lhs, const wchar_t* rhs) {
+ if (lhs.IsEmpty()) {
+ return !rhs || !rhs[0];
}
- if (!ptr) {
- return data_->data_length_ == 0;
- }
-
- // SAFTEY: `wsclen()` comparison (whose own safety depends upoon the caller)
- // ensures there are `data_length_` wchars at `ptr` before the terminator,
- // and `data_length_` is within `string_`.
- return UNSAFE_BUFFERS(wcslen(ptr)) == data_->data_length_ &&
- UNSAFE_BUFFERS(
- FXSYS_wmemcmp(ptr, data_->string_, data_->data_length_)) == 0;
-}
-
-bool WideString::operator==(WideStringView str) const {
- if (!data_) {
- return str.IsEmpty();
- }
-
- // SAFTEY: Comparison ensure there are `data_length_` wchars in `str`
- // and `data_length_ is within `string_`.
- return data_->data_length_ == str.GetLength() &&
- UNSAFE_BUFFERS(FXSYS_wmemcmp(data_->string_, str.unterminated_c_str(),
- str.GetLength())) == 0;
-}
-
-bool WideString::operator==(const WideString& other) const {
- if (data_ == other.data_) {
- return true;
- }
-
- if (IsEmpty()) {
- return other.IsEmpty();
- }
-
- if (other.IsEmpty()) {
+ if (!rhs) {
return false;
}
- // SAFETY: data_length_ bytes available at string_.
- return other.data_->data_length_ == data_->data_length_ &&
- UNSAFE_BUFFERS(FXSYS_wmemcmp(other.data_->string_, data_->string_,
- data_->data_length_)) == 0;
+ // SAFTEY: required from caller.
+ return UNSAFE_BUFFERS(wcscmp(lhs.data_->string_, rhs)) == 0;
}
bool WideString::operator<(const wchar_t* ptr) const {
diff --git a/core/fxcrt/widestring.h b/core/fxcrt/widestring.h
index 2e7345b..7dc81c4 100644
--- a/core/fxcrt/widestring.h
+++ b/core/fxcrt/widestring.h
@@ -76,13 +76,14 @@
WideString& operator+=(const WideString& str);
WideString& operator+=(WideStringView str);
- bool operator==(const wchar_t* ptr) const;
- bool operator==(WideStringView str) const;
- bool operator==(const WideString& other) const;
-
- bool operator!=(const wchar_t* ptr) const { return !(*this == ptr); }
- bool operator!=(WideStringView str) const { return !(*this == str); }
- bool operator!=(const WideString& other) const { return !(*this == other); }
+ // TODO(tsepez): The wchar_t* form should be UNSAFE_BUFFER_USAGE.
+ friend bool operator==(const WideString& lhs, const wchar_t* rhs);
+ friend bool operator==(const WideString& lhs, WideStringView rhs) {
+ return lhs.AsStringView() == rhs;
+ }
+ friend bool operator==(const WideString& lhs, const WideString& rhs) {
+ return lhs.AsStringView() == rhs.AsStringView();
+ }
bool operator<(const wchar_t* ptr) const;
bool operator<(WideStringView str) const;
@@ -174,18 +175,6 @@
inline WideString operator+(WideStringView str1, const WideString& str2) {
return WideString(str1, str2.AsStringView());
}
-inline bool operator==(const wchar_t* lhs, const WideString& rhs) {
- return rhs == lhs;
-}
-inline bool operator==(WideStringView lhs, const WideString& rhs) {
- return rhs == lhs;
-}
-inline bool operator!=(const wchar_t* lhs, const WideString& rhs) {
- return rhs != lhs;
-}
-inline bool operator!=(WideStringView lhs, const WideString& rhs) {
- return rhs != lhs;
-}
inline bool operator<(const wchar_t* lhs, const WideString& rhs) {
return rhs.Compare(lhs) > 0;
}
