Skip to content

Commit

Permalink
Cosmetics:
Browse files Browse the repository at this point in the history
* When verifying the postcondition that reading/writing functions did not
  read/write more than requested, check exactly against what was requested,
  rather than against a potentially greater value.

* Rely on a condition which was asserted before.

PiperOrigin-RevId: 642192963
  • Loading branch information
QrczakMK committed Jun 11, 2024
1 parent bb038c1 commit d2b5457
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 15 deletions.
8 changes: 4 additions & 4 deletions python/riegeli/bytes/python_writer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -144,11 +144,11 @@ bool PythonWriter::WriteInternal(absl::string_view src) {
}
}
do {
const size_t length_to_write = UnsignedMin(
src.size(),
absl::bit_floor(size_t{std::numeric_limits<Py_ssize_t>::max()}));
size_t length_written;
{
const size_t length_to_write = UnsignedMin(
src.size(),
absl::bit_floor(size_t{std::numeric_limits<Py_ssize_t>::max()}));
PythonPtr write_result;
if (!use_bytes_) {
// Prefer passing a `memoryview` to avoid copying memory.
Expand Down Expand Up @@ -198,7 +198,7 @@ bool PythonWriter::WriteInternal(absl::string_view src) {
length_written = *length_written_opt;
}
}
if (ABSL_PREDICT_FALSE(length_written > src.size())) {
if (ABSL_PREDICT_FALSE(length_written > length_to_write)) {
return Fail(absl::InternalError("write() wrote more than requested"));
}
move_start_pos(length_written);
Expand Down
2 changes: 1 addition & 1 deletion riegeli/bytes/cfile_reader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ bool CFileReaderBase::ReadInternal(size_t min_length, size_t max_length,
<< "fread() succeeded but read less than requested";
clearerr(src);
if (!growing_source_) set_exact_size(limit_pos());
return length_read >= min_length;
return false;
}
if (length_read >= min_length) return true;
dest += length_read;
Expand Down
12 changes: 7 additions & 5 deletions riegeli/bytes/fd_reader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -410,18 +410,19 @@ bool FdReaderBase::ReadInternal(size_t min_length, size_t max_length,
return FailOperation(has_independent_pos_ ? "pread()" : "read()");
}
#else // _WIN32
DWORD length_to_read;
DWORD length_read;
if (has_independent_pos_) {
const HANDLE file_handle = reinterpret_cast<HANDLE>(_get_osfhandle(src));
if (ABSL_PREDICT_FALSE(file_handle == INVALID_HANDLE_VALUE ||
file_handle == reinterpret_cast<HANDLE>(-2))) {
return FailWindowsOperation("_get_osfhandle()");
}
const DWORD length_to_read = IntCast<DWORD>(UnsignedMin(
length_to_read = UnsignedMin(
max_length,
Position{std::numeric_limits<fd_internal::Offset>::max()} -
limit_pos(),
absl::bit_floor(std::numeric_limits<DWORD>::max())));
absl::bit_floor(std::numeric_limits<DWORD>::max()));
OVERLAPPED overlapped{};
overlapped.Offset = IntCast<DWORD>(limit_pos() & 0xffffffff);
overlapped.OffsetHigh = IntCast<DWORD>(limit_pos() >> 32);
Expand All @@ -431,12 +432,13 @@ bool FdReaderBase::ReadInternal(size_t min_length, size_t max_length,
return FailWindowsOperation("ReadFile()");
}
} else {
const unsigned length_to_read = UnsignedMin(
length_to_read = UnsignedMin(
max_length,
Position{std::numeric_limits<fd_internal::Offset>::max()} -
limit_pos(),
absl::bit_floor(unsigned{std::numeric_limits<int>::max()}));
const int length_read_int = _read(src, dest, length_to_read);
const int length_read_int =
_read(src, dest, IntCast<unsigned>(length_to_read));
if (ABSL_PREDICT_FALSE(length_read_int < 0)) {
return FailOperation("_read()");
}
Expand All @@ -447,7 +449,7 @@ bool FdReaderBase::ReadInternal(size_t min_length, size_t max_length,
if (!growing_source_) set_exact_size(limit_pos());
return false;
}
RIEGELI_ASSERT_LE(IntCast<size_t>(length_read), max_length)
RIEGELI_ASSERT_LE(UnsignedCast(length_read), length_to_read)
#ifndef _WIN32
<< (has_independent_pos_ ? "pread()" : "read()")
#else
Expand Down
10 changes: 6 additions & 4 deletions riegeli/bytes/fd_writer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -481,14 +481,15 @@ bool FdWriterBase::WriteInternal(absl::string_view src) {
return FailOperation(has_independent_pos_ ? "pwrite()" : "write()");
}
#else // _WIN32
DWORD length_to_write;
DWORD length_written;
if (has_independent_pos_) {
const HANDLE file_handle = reinterpret_cast<HANDLE>(_get_osfhandle(dest));
if (ABSL_PREDICT_FALSE(file_handle == INVALID_HANDLE_VALUE ||
file_handle == reinterpret_cast<HANDLE>(-2))) {
return FailWindowsOperation("_get_osfhandle()");
}
const DWORD length_to_write = UnsignedMin(
length_to_write = UnsignedMin(
src.size(), absl::bit_floor(std::numeric_limits<DWORD>::max()));
OVERLAPPED overlapped{};
overlapped.Offset = IntCast<DWORD>(start_pos() & 0xffffffff);
Expand All @@ -499,10 +500,11 @@ bool FdWriterBase::WriteInternal(absl::string_view src) {
return FailWindowsOperation("WriteFile()");
}
} else {
const unsigned length_to_write = UnsignedMin(
length_to_write = UnsignedMin(
src.size(),
absl::bit_floor(unsigned{std::numeric_limits<int>::max()}));
const int length_written_int = _write(dest, src.data(), length_to_write);
const int length_written_int =
_write(dest, src.data(), IntCast<unsigned>(length_to_write));
if (ABSL_PREDICT_FALSE(length_written_int < 0)) {
return FailOperation("_write()");
}
Expand All @@ -516,7 +518,7 @@ bool FdWriterBase::WriteInternal(absl::string_view src) {
<< (has_independent_pos_ ? "WriteFile()" : "_write()")
#endif
<< " returned 0";
RIEGELI_ASSERT_LE(IntCast<size_t>(length_written), src.size())
RIEGELI_ASSERT_LE(UnsignedCast(length_written), length_to_write)
#ifndef _WIN32
<< (has_independent_pos_ ? "pwrite()" : "write()")
#else
Expand Down
2 changes: 1 addition & 1 deletion riegeli/bytes/istream_reader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ bool IStreamReaderBase::ReadInternal(size_t min_length, size_t max_length,
}
length_read = src.readsome(dest, max_length_to_read);
RIEGELI_ASSERT_GE(length_read, 0) << "negative istream::readsome()";
RIEGELI_ASSERT_LE(IntCast<size_t>(length_read), max_length)
RIEGELI_ASSERT_LE(length_read, max_length_to_read)
<< "istream::readsome() read more than requested";
if (ABSL_PREDICT_TRUE(length_read > 0)) goto fragment_read;
// `std::istream::peek()` returned non-`eof()` but
Expand Down

0 comments on commit d2b5457

Please sign in to comment.