From b20419efaf58ae6da2aed9f115961bfcefda40a9 Mon Sep 17 00:00:00 2001 From: Tim Wojtulewicz Date: Thu, 17 Apr 2025 13:23:37 -0700 Subject: [PATCH] Fix clang-tidy bugprone-suspicious-realloc-usage warnings --- .clang-tidy | 1 + src/Reporter.cc | 51 +++++++++++++++++++++++++++++++------------------ 2 files changed, 33 insertions(+), 19 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 3b8a2139c1..d7ebf7a090 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -13,4 +13,5 @@ Checks: [-*, bugprone-suspicious-stringview-data-usage, bugprone-suspicious-string-compare, bugprone-suspicious-include, + bugprone-suspicious-realloc-usage, ] diff --git a/src/Reporter.cc b/src/Reporter.cc index 217fd277b2..43510b315f 100644 --- a/src/Reporter.cc +++ b/src/Reporter.cc @@ -475,7 +475,8 @@ void Reporter::Deprecation(std::string_view msg, const detail::Location* loc1, c void Reporter::DoLog(const char* prefix, EventHandlerPtr event, FILE* out, Connection* conn, ValPList* addl, bool location, bool time, const char* postfix, const char* fmt, va_list ap) { - static char tmp[512]; + constexpr size_t DEFAULT_BUFFER_SIZE = 512; + static char tmp[DEFAULT_BUFFER_SIZE]; int size = sizeof(tmp); char* buffer = tmp; @@ -522,27 +523,38 @@ void Reporter::DoLog(const char* prefix, EventHandlerPtr event, FILE* out, Conne } } - while ( true ) { - va_list aq; - va_copy(aq, ap); - int n = vsnprintf(buffer, size, fmt, aq); - va_end(aq); + // Figure out how big of a buffer is needed here. + va_list ap_copy; + va_copy(ap_copy, ap); + size_t req_buffer_size = vsnprintf(nullptr, 0, fmt, ap_copy); - if ( postfix ) - n += strlen(postfix) + 10; // Add a bit of slack. - - if ( n > -1 && n < size ) - // We had enough space; - break; - - // Enlarge buffer; - size *= 2; - buffer = allocated = (char*)realloc(allocated, size); - - if ( ! buffer ) - FatalError("out of memory in Reporter"); + if ( req_buffer_size < 0 ) { + va_end(ap_copy); + FatalError("out of memory in Reporter"); } + if ( postfix && *postfix ) + req_buffer_size += strlen(postfix) + 10; + + // Add one byte for a null terminator. + req_buffer_size++; + + if ( req_buffer_size > DEFAULT_BUFFER_SIZE ) { + buffer = (char*)malloc(req_buffer_size); + if ( ! buffer ) { + va_end(ap_copy); + FatalError("out of memory in Reporter"); + } + + allocated = buffer; + } + + va_end(ap_copy); + + req_buffer_size = vsnprintf(buffer, req_buffer_size, fmt, ap); + if ( req_buffer_size < 0 ) + FatalError("out of memory in Reporter"); + if ( postfix && *postfix ) // Note, if you change this fmt string, adjust the additional // buffer size above. @@ -641,6 +653,7 @@ void Reporter::DoLog(const char* prefix, EventHandlerPtr event, FILE* out, Conne #endif } + // If the buffer got reallocated because it was too small, free the reallocated one. if ( allocated ) free(allocated); }