Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions absl/base/internal/raw_logging.cc
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ bool VADoRawLog(char** buf, int* size, const char* format, va_list ap) {
if (*size < 0) return false;
int n = vsnprintf(*buf, static_cast<size_t>(*size), format, ap);
bool result = true;
if (n < 0 || n > *size) {
if (n < 0 || n >= *size) {
result = false;
if (static_cast<size_t>(*size) > sizeof(kTruncated)) {
n = *size - static_cast<int>(sizeof(kTruncated));
Expand Down Expand Up @@ -125,7 +125,7 @@ bool DoRawLog(char** buf, int* size, const char* format, ...) {
va_start(ap, format);
int n = vsnprintf(*buf, static_cast<size_t>(*size), format, ap);
va_end(ap);
if (n < 0 || n > *size) return false;
if (n < 0 || n >= *size) return false;
*size -= n;
*buf += n;
return true;
Expand Down
25 changes: 25 additions & 0 deletions absl/base/raw_logging_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,31 @@ TEST(RawLoggingDeathTest, LogFatal) {
kExpectedDeathOutput);
}

// A message whose formatted length exactly equals the remaining buffer size is
// still truncated: vsnprintf() only writes size-1 characters plus the NUL, so
// truncation must be detected when the would-be length is >= the size, not only
// when it is strictly greater. With the off-by-one, such a message loses its
// final character and is emitted without the "(message truncated)" marker (and
// without the trailing newline). A custom zero-length prefix hook makes the
// whole raw-log buffer available so the boundary is hit deterministically.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is describing the old buggy state, and isn't particularly useful. I'd just delete the whole thing

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call, dropped it.

TEST(RawLoggingDeathTest, TruncationMarkerAtExactBufferBoundary) {
if (!absl::raw_log_internal::RawLoggingFullySupported()) {
GTEST_SKIP() << "Raw logging output is not supported on this platform.";
}
EXPECT_DEATH_IF_SUPPORTED(
[] {
absl::raw_log_internal::RegisterLogFilterAndPrefixHook(
[](absl::LogSeverity, const char*, int, char**, int*) {
return true; // Enable logging, write no prefix.
});
// kLogBufSize in raw_logging.cc is 3000; a message of exactly that many
// characters fills the buffer and must be reported as truncated.
const std::string msg(3000, 'x');
ABSL_RAW_LOG(FATAL, "%s", msg.c_str());
}(),
"message truncated");
}

TEST(InternalLog, CompilationTest) {
ABSL_INTERNAL_LOG(INFO, "Internal Log");
std::string log_msg = "Internal Log";
Expand Down