Skip to content

Commit

Permalink
Do not bother checking future std::string and absl::string_view s…
Browse files Browse the repository at this point in the history 
…izes

against `max_size()`.

`std::string` does this itself by throwing an exception, and `absl::string_view`
close to its `max_size()` is infeasible anyway.

This is simpler and cheaper.

Check future `std::string` sizes against `std::numeric_limits<size_t>::max()`
though to protect against arithmetic overflow before a `std::string` would be
resized and would have a chance to check the size itself.

Keep checks against `max_size()` when exceeding the size should better not be
fatal, even though it is unlikely that we can get close to that range.

PiperOrigin-RevId: 649054068
  • Loading branch information
@QrczakMK
QrczakMK committed Jul 3, 2024
1 parent 0a7d9ea commit 749f2e8
Show file tree
Hide file tree
Showing 7 changed files with 39 additions and 63 deletions.
13 changes: 5 additions & 8 deletions riegeli/base/chain.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -587,9 +587,6 @@ inline void Chain::Initialize(const Chain& src) {
inline std::string Chain::ToString() const {
if (begin_ == end_) return std::string(short_data());
std::string dest;
RIEGELI_CHECK_LE(size_, dest.max_size())
<< "Failed precondition of Chain::operator string: "
"string size overflow";
dest.resize(size_);
CopyToSlow(&dest[0]);
return dest;
Expand Down Expand Up @@ -673,18 +670,14 @@ inline void Chain::CopyToSlow(char* dest) const {

void Chain::AppendTo(std::string& dest) const& {
const size_t size_before = dest.size();
RIEGELI_CHECK_LE(size_, dest.max_size() - size_before)
RIEGELI_CHECK_LE(size_, std::numeric_limits<size_t>::max() - size_before)
<< "Failed precondition of Chain::AppendTo(string&): "
"string size overflow";
ResizeStringAmortized(dest, size_before + size_);
CopyTo(&dest[size_before]);
}

void Chain::AppendTo(std::string& dest) && {
const size_t size_before = dest.size();
RIEGELI_CHECK_LE(size_, dest.max_size() - size_before)
<< "Failed precondition of Chain::AppendTo(string&): "
"string size overflow";
if (dest.empty() && PtrDistance(begin_, end_) == 1) {
if (std::string* const string_ptr =
back()->checked_external_object_with_unique_owner<std::string>()) {
Expand All @@ -699,6 +692,10 @@ void Chain::AppendTo(std::string& dest) && {
}
}
}
const size_t size_before = dest.size();
RIEGELI_CHECK_LE(size_, std::numeric_limits<size_t>::max() - size_before)
<< "Failed precondition of Chain::AppendTo(string&): "
"string size overflow";
ResizeStringAmortized(dest, size_before + size_);
CopyTo(&dest[size_before]);
}
Expand Down
2 changes: 0 additions & 2 deletions riegeli/bytes/read_all.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ ABSL_ATTRIBUTE_COLD absl::Status MaxLengthExceeded(Reader& src,

absl::Status ReadAllImpl(Reader& src, absl::string_view& dest,
size_t max_length) {
max_length = UnsignedMin(max_length, dest.max_size());
if (src.SupportsSize()) {
const absl::optional<Position> size = src.Size();
if (ABSL_PREDICT_FALSE(size == absl::nullopt)) {
Expand Down Expand Up @@ -81,7 +80,6 @@ absl::Status ReadAllImpl(Reader& src, absl::string_view& dest,

absl::Status ReadAndAppendAllImpl(Reader& src, std::string& dest,
size_t max_length) {
max_length = UnsignedMin(max_length, dest.max_size() - dest.size());
if (src.SupportsSize()) {
const absl::optional<Position> size = src.Size();
if (ABSL_PREDICT_FALSE(size == absl::nullopt)) return src.status();
Expand Down
9 changes: 5 additions & 4 deletions riegeli/bytes/reader.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,10 +111,10 @@ bool Reader::ReadSlow(size_t length, std::string& dest) {
RIEGELI_ASSERT_LT(available(), length)
<< "Failed precondition of Reader::ReadSlow(string&): "
"enough data available, use Read(string&) instead";
RIEGELI_CHECK_LE(length, dest.max_size() - dest.size())
const size_t dest_pos = dest.size();
RIEGELI_CHECK_LE(length, std::numeric_limits<size_t>::max() - dest_pos)
<< "Failed precondition of Reader::ReadSlow(string&): "
"string size overflow";
const size_t dest_pos = dest.size();
ResizeStringAmortized(dest, dest_pos + length);
size_t length_read;
if (ABSL_PREDICT_FALSE(!ReadSlow(length, &dest[dest_pos], length_read))) {
Expand All @@ -128,7 +128,7 @@ bool Reader::ReadSlow(size_t length, std::string& dest, size_t& length_read) {
RIEGELI_ASSERT_LT(available(), length)
<< "Failed precondition of Reader::ReadSlow(string&): "
"enough data available, use Read(string&) instead";
RIEGELI_ASSERT_LE(length, dest.max_size() - dest.size())
RIEGELI_ASSERT_LE(length, std::numeric_limits<size_t>::max() - dest.size())
<< "Failed precondition of Reader::ReadSlow(string&): "
"string size overflow";
const Position pos_before = pos();
Expand Down Expand Up @@ -377,7 +377,8 @@ bool Reader::ReadSomeSlow(size_t max_length, std::string& dest) {
<< "Failed precondition of Reader::ReadSomeSlow(string&): "
"enough data available, use ReadSome(string&) instead";
const size_t dest_size_before = dest.size();
const size_t remaining = dest.max_size() - dest_size_before;
const size_t remaining =
std::numeric_limits<size_t>::max() - dest_size_before;
RIEGELI_CHECK_GT(remaining, 0u)
<< "Failed precondition of Reader::ReadSome(string&): "
"string size overflow";
Expand Down
40 changes: 12 additions & 28 deletions riegeli/bytes/reader.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -178,9 +178,6 @@ class Reader : public Object {
// This is equal to the difference between `pos()` after and before the call,
// and is equal to `length` if `Read()` returned `true`.
//
// Precondition for `Read(std::string&)`:
// `length <= dest.max_size()`
//
// Return values:
// * `true` - success (`length` bytes read)
// * `false` (when `ok()`) - source ends (less than `length` bytes read)
Expand All @@ -201,10 +198,8 @@ class Reader : public Object {
// before the call, and is equal to `length` if `ReadAndAppend()` returned
// `true`.
//
// Precondition for `ReadAndAppend(std::string&)`:
// `length <= dest->max_size() - dest->size()`
//
// Precondition for `ReadAndAppend(Chain&)` and `ReadAndAppend(absl::Cord&)`:
// Precondition for `ReadAndAppend(std::string&)`, `ReadAndAppend(Chain&)`,
// and `ReadAndAppend(absl::Cord&)`:
// `length <= std::numeric_limits<size_t>::max() - dest->size()`
//
// Return values:
Expand Down Expand Up @@ -271,11 +266,8 @@ class Reader : public Object {
// If `length_read != nullptr` then sets `*length_read` to the length read.
// This is equal to the difference between `pos()` after and before the call.
//
// Precondition for `ReadAndAppendSome(std::string&)`:
// `max_length == 0 || dest->size() < dest->max_size()`
//
// Precondition for `ReadAndAppendSome(Chain&)` and
// `ReadAndAppend(absl::Cord&)`:
// Precondition for `ReadAndAppendSome(std::string&)`,
// `ReadAndAppendSome(Chain&)`, and `ReadAndAppend(absl::Cord&)`:
// `max_length == 0 || dest->size() < std::numeric_limits<size_t>::max()`
//
// Return values:
Expand Down Expand Up @@ -594,17 +586,12 @@ class Reader : public Object {
// `max_length > 0`
// `available() > 0`
//
// Additional precondition for `ReadSlow(std::string&)`:
// `length <= dest->max_size() - dest->size()`
//
// Additional precondition for `ReadSlow(Chain&)` and `ReadSlow(absl::Cord&)`:
// Additional precondition for `ReadSlow(std::string&)`, `ReadSlow(Chain&)`,
// and `ReadSlow(absl::Cord&)`:
// `length <= std::numeric_limits<size_t>::max() - dest->size()`
//
// `ReadSome()` preconditions checked by `ReadSomeSlow(std::string&)`:
// `dest->size() < dest->max_size()`
//
// `ReadSome()` preconditions checked by `ReadSomeSlow(Chain&)` and
// `ReadSomeSlow(absl::Cord&)`:
// `ReadSome()` preconditions checked by `ReadSomeSlow(std::string&)`,
// `ReadSomeSlow(Chain&)`, and `ReadSomeSlow(absl::Cord&)`:
// `dest->size() < std::numeric_limits<size_t>::max()`
virtual bool ReadSlow(size_t length, char* dest);
bool ReadSlow(size_t length, char* dest, size_t& length_read);
Expand Down Expand Up @@ -808,9 +795,6 @@ inline bool Reader::Read(size_t length, char* dest, size_t* length_read) {

inline bool Reader::Read(size_t length, std::string& dest,
size_t* length_read) {
RIEGELI_ASSERT_LE(length, dest.max_size())
<< "Failed precondition of Reader::Read(string&): "
"string size overflow";
if (ABSL_PREDICT_TRUE(available() >= length)) {
// `std::string::assign()` checks for size overflow.
dest.assign(cursor(), length);
Expand Down Expand Up @@ -849,7 +833,7 @@ inline bool Reader::Read(size_t length, absl::Cord& dest, size_t* length_read) {

inline bool Reader::ReadAndAppend(size_t length, std::string& dest,
size_t* length_read) {
RIEGELI_ASSERT_LE(length, dest.max_size() - dest.size())
RIEGELI_ASSERT_LE(length, std::numeric_limits<size_t>::max() - dest.size())
<< "Failed precondition of Reader::ReadAndAppend(string&): "
"string size overflow";
if (ABSL_PREDICT_TRUE(available() >= length)) {
Expand Down Expand Up @@ -952,8 +936,7 @@ inline bool Reader::ReadSome(size_t max_length, char* dest,

inline bool Reader::ReadSome(size_t max_length, std::string& dest,
size_t* length_read) {
if (ABSL_PREDICT_TRUE(available() >= max_length &&
max_length <= dest.max_size())) {
if (ABSL_PREDICT_TRUE(available() >= max_length)) {
dest.assign(cursor(), max_length);
move_cursor(max_length);
if (length_read != nullptr) *length_read = max_length;
Expand Down Expand Up @@ -1001,7 +984,8 @@ inline bool Reader::ReadSome(size_t max_length, absl::Cord& dest,
inline bool Reader::ReadAndAppendSome(size_t max_length, std::string& dest,
size_t* length_read) {
if (ABSL_PREDICT_TRUE(available() >= max_length &&
max_length <= dest.max_size() - dest.size())) {
max_length <=
std::numeric_limits<size_t>::max() - dest.size())) {
dest.append(cursor(), max_length);
move_cursor(max_length);
if (length_read != nullptr) *length_read = max_length;
Expand Down
30 changes: 15 additions & 15 deletions riegeli/bytes/string_writer.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,7 @@ void StringWriterBase::SetWriteSizeHintImpl(
RIEGELI_ASSERT_EQ(UnsignedMax(limit_pos(), written_size_),
dest.size() + secondary_buffer_.size())
<< "StringWriter destination changed unexpectedly";
const size_t size_hint =
UnsignedMin(SaturatingAdd(pos(), *write_size_hint), dest.max_size());
const size_t size_hint = SaturatingAdd(pos(), *write_size_hint);
if (!uses_secondary_buffer()) {
SyncDestBuffer(dest);
if (dest.capacity() < size_hint) dest.reserve(size_hint);
Expand All @@ -121,8 +120,8 @@ bool StringWriterBase::PushSlow(size_t min_length, size_t recommended_length) {
RIEGELI_ASSERT_EQ(UnsignedMax(limit_pos(), written_size_),
dest.size() + secondary_buffer_.size())
<< "StringWriter destination changed unexpectedly";
if (ABSL_PREDICT_FALSE(min_length >
dest.max_size() - IntCast<size_t>(pos()))) {
if (ABSL_PREDICT_FALSE(min_length > std::numeric_limits<size_t>::max() -
IntCast<size_t>(pos()))) {
return FailOverflow();
}
if (!uses_secondary_buffer()) {
Expand Down Expand Up @@ -160,8 +159,8 @@ bool StringWriterBase::WriteSlow(const Chain& src) {
RIEGELI_ASSERT_EQ(UnsignedMax(limit_pos(), written_size_),
dest.size() + secondary_buffer_.size())
<< "StringWriter destination changed unexpectedly";
if (ABSL_PREDICT_FALSE(src.size() >
dest.max_size() - IntCast<size_t>(pos()))) {
if (ABSL_PREDICT_FALSE(src.size() > std::numeric_limits<size_t>::max() -
IntCast<size_t>(pos()))) {
return FailOverflow();
}
if (!uses_secondary_buffer()) {
Expand Down Expand Up @@ -198,8 +197,8 @@ bool StringWriterBase::WriteSlow(Chain&& src) {
RIEGELI_ASSERT_EQ(UnsignedMax(limit_pos(), written_size_),
dest.size() + secondary_buffer_.size())
<< "StringWriter destination changed unexpectedly";
if (ABSL_PREDICT_FALSE(src.size() >
dest.max_size() - IntCast<size_t>(pos()))) {
if (ABSL_PREDICT_FALSE(src.size() > std::numeric_limits<size_t>::max() -
IntCast<size_t>(pos()))) {
return FailOverflow();
}
if (!uses_secondary_buffer()) {
Expand Down Expand Up @@ -236,8 +235,8 @@ bool StringWriterBase::WriteSlow(const absl::Cord& src) {
RIEGELI_ASSERT_EQ(UnsignedMax(limit_pos(), written_size_),
dest.size() + secondary_buffer_.size())
<< "StringWriter destination changed unexpectedly";
if (ABSL_PREDICT_FALSE(src.size() >
dest.max_size() - IntCast<size_t>(pos()))) {
if (ABSL_PREDICT_FALSE(src.size() > std::numeric_limits<size_t>::max() -
IntCast<size_t>(pos()))) {
return FailOverflow();
}
if (!uses_secondary_buffer()) {
Expand Down Expand Up @@ -274,8 +273,8 @@ bool StringWriterBase::WriteSlow(absl::Cord&& src) {
RIEGELI_ASSERT_EQ(UnsignedMax(limit_pos(), written_size_),
dest.size() + secondary_buffer_.size())
<< "StringWriter destination changed unexpectedly";
if (ABSL_PREDICT_FALSE(src.size() >
dest.max_size() - IntCast<size_t>(pos()))) {
if (ABSL_PREDICT_FALSE(src.size() > std::numeric_limits<size_t>::max() -
IntCast<size_t>(pos()))) {
return FailOverflow();
}
if (!uses_secondary_buffer()) {
Expand Down Expand Up @@ -312,8 +311,8 @@ bool StringWriterBase::WriteSlow(ExternalRef src) {
RIEGELI_ASSERT_EQ(UnsignedMax(limit_pos(), written_size_),
dest.size() + secondary_buffer_.size())
<< "StringWriter destination changed unexpectedly";
if (ABSL_PREDICT_FALSE(src.size() >
dest.max_size() - IntCast<size_t>(pos()))) {
if (ABSL_PREDICT_FALSE(src.size() > std::numeric_limits<size_t>::max() -
IntCast<size_t>(pos()))) {
return FailOverflow();
}
if (!uses_secondary_buffer()) {
Expand Down Expand Up @@ -353,7 +352,8 @@ bool StringWriterBase::WriteZerosSlow(Position length) {
RIEGELI_ASSERT_EQ(UnsignedMax(limit_pos(), written_size_),
dest.size() + secondary_buffer_.size())
<< "StringWriter destination changed unexpectedly";
if (ABSL_PREDICT_FALSE(length > dest.max_size() - IntCast<size_t>(pos()))) {
if (ABSL_PREDICT_FALSE(length > std::numeric_limits<size_t>::max() -
IntCast<size_t>(pos()))) {
return FailOverflow();
}
if (!uses_secondary_buffer()) {
Expand Down
6 changes: 2 additions & 4 deletions riegeli/csv/csv_reader.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,10 +151,8 @@ void CsvReaderBase::Initialize(Reader* src, Options&& options) {
}
skip_empty_lines_ = options.skip_empty_lines();
quote_ = options.quote().value_or('\0');
max_num_fields_ = UnsignedMin(options.max_num_fields(),
std::vector<std::string>().max_size());
max_field_length_ =
UnsignedMin(options.max_field_length(), std::string().max_size());
max_num_fields_ = options.max_num_fields();
max_field_length_ = options.max_field_length();

if (ABSL_PREDICT_FALSE(!src->ok())) {
FailWithoutAnnotation(AnnotateOverSrc(src->status()));
Expand Down
2 changes: 0 additions & 2 deletions riegeli/lines/line_reading.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,6 @@ inline bool ReadLineInternal(Reader& src, Dest& dest, ReadLineOptions options) {
} // namespace

bool ReadLine(Reader& src, absl::string_view& dest, ReadLineOptions options) {
options.set_max_length(UnsignedMin(options.max_length(), dest.max_size()));
size_t length = 0;
if (ABSL_PREDICT_FALSE(!src.Pull())) {
dest = absl::string_view();
Expand Down Expand Up @@ -293,7 +292,6 @@ bool ReadLine(Reader& src, absl::string_view& dest, ReadLineOptions options) {

bool ReadLine(Reader& src, std::string& dest, ReadLineOptions options) {
dest.clear();
options.set_max_length(UnsignedMin(options.max_length(), dest.max_size()));
return ReadLineInternal(src, dest, options);
}

Expand Down

0 comments on commit 749f2e8

Please sign in to comment.