From bc6d88fabe7505f43f0941f3883135523a96e9dc Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 2 Feb 2026 07:57:00 +0100 Subject: [PATCH] [logger] Use vsnprintf_P directly for ESP8266 flash format strings Instead of copying the format string from flash to RAM before formatting, use vsnprintf_P to read the format string directly from flash memory. This eliminates: - The byte-by-byte copy loop from PROGMEM - The complex dual-purpose buffer management - Potential buffer overflow if format string is very long The new format_body_to_buffer_P_() function is a simple variant that uses vsnprintf_P instead of vsnprintf. --- esphome/components/logger/logger.cpp | 50 ++++++++-------------------- esphome/components/logger/logger.h | 23 +++++++++++++ 2 files changed, 37 insertions(+), 36 deletions(-) diff --git a/esphome/components/logger/logger.cpp b/esphome/components/logger/logger.cpp index 3a726d4046..e7eba8fd45 100644 --- a/esphome/components/logger/logger.cpp +++ b/esphome/components/logger/logger.cpp @@ -128,22 +128,7 @@ void HOT Logger::log_vprintf_(uint8_t level, const char *tag, int line, const ch // Note: USE_STORE_LOG_STR_IN_FLASH is only defined for ESP8266. // // This function handles format strings stored in flash memory (PROGMEM) to save RAM. -// The buffer is used in a special way to avoid allocating extra memory: -// -// Memory layout during execution: -// Step 1: Copy format string from flash to buffer -// tx_buffer_: [format_string][null][.....................] -// tx_buffer_at_: ------------------^ -// msg_start: saved here -----------^ -// -// Step 2: format_log_to_buffer_with_terminator_ reads format string from beginning -// and writes formatted output starting at msg_start position -// tx_buffer_: [format_string][null][formatted_message][null] -// tx_buffer_at_: -------------------------------------^ -// -// Step 3: Output the formatted message (starting at msg_start) -// write_msg_ and callbacks receive: this->tx_buffer_ + msg_start -// which points to: [formatted_message][null] +// Uses vsnprintf_P to read the format string directly from flash without copying to RAM. // void Logger::log_vprintf_(uint8_t level, const char *tag, int line, const __FlashStringHelper *format, va_list args) { // NOLINT @@ -153,35 +138,28 @@ void Logger::log_vprintf_(uint8_t level, const char *tag, int line, const __Flas RecursionGuard guard(global_recursion_guard_); this->tx_buffer_at_ = 0; - // Copy format string from progmem - auto *format_pgm_p = reinterpret_cast(format); - char ch = '.'; - while (this->tx_buffer_at_ < this->tx_buffer_size_ && ch != '\0') { - this->tx_buffer_[this->tx_buffer_at_++] = ch = (char) progmem_read_byte(format_pgm_p++); - } + // Write header, format body directly from flash, and write footer + this->write_header_to_buffer_(level, tag, line, nullptr, this->tx_buffer_, &this->tx_buffer_at_, + this->tx_buffer_size_); + this->format_body_to_buffer_P_(this->tx_buffer_, &this->tx_buffer_at_, this->tx_buffer_size_, + reinterpret_cast(format), args); + this->write_footer_to_buffer_(this->tx_buffer_, &this->tx_buffer_at_, this->tx_buffer_size_); - // Buffer full from copying format - RAII guard handles cleanup on return + // Ensure null termination if (this->tx_buffer_at_ >= this->tx_buffer_size_) { - return; + this->tx_buffer_[this->tx_buffer_size_ - 1] = '\0'; + } else { + this->tx_buffer_[this->tx_buffer_at_] = '\0'; } - // Save the offset before calling format_log_to_buffer_with_terminator_ - // since it will increment tx_buffer_at_ to the end of the formatted string - uint16_t msg_start = this->tx_buffer_at_; - this->format_log_to_buffer_with_terminator_(level, tag, line, this->tx_buffer_, args, this->tx_buffer_, - &this->tx_buffer_at_, this->tx_buffer_size_); - - uint16_t msg_length = - this->tx_buffer_at_ - msg_start; // Don't subtract 1 - tx_buffer_at_ is already at the null terminator position - // Listeners get message first (before console write) #ifdef USE_LOG_LISTENERS for (auto *listener : this->log_listeners_) - listener->on_log(level, tag, this->tx_buffer_ + msg_start, msg_length); + listener->on_log(level, tag, this->tx_buffer_, this->tx_buffer_at_); #endif - // Write to console starting at the msg_start - this->write_tx_buffer_to_console_(msg_start, &msg_length); + // Write to console + this->write_tx_buffer_to_console_(); } #endif // USE_STORE_LOG_STR_IN_FLASH diff --git a/esphome/components/logger/logger.h b/esphome/components/logger/logger.h index fe9cab4993..7e41030b19 100644 --- a/esphome/components/logger/logger.h +++ b/esphome/components/logger/logger.h @@ -622,6 +622,29 @@ class Logger : public Component { } } +#ifdef USE_STORE_LOG_STR_IN_FLASH + // ESP8266 variant that reads format string directly from flash using vsnprintf_P + inline void HOT format_body_to_buffer_P_(char *buffer, uint16_t *buffer_at, uint16_t buffer_size, PGM_P format, + va_list args) { + if (*buffer_at >= buffer_size) + return; + const uint16_t remaining = buffer_size - *buffer_at; + + const int ret = vsnprintf_P(buffer + *buffer_at, remaining, format, args); + + if (ret < 0) { + return; + } + + uint16_t formatted_len = (ret >= remaining) ? (remaining - 1) : ret; + *buffer_at += formatted_len; + + while (*buffer_at > 0 && buffer[*buffer_at - 1] == '\n') { + (*buffer_at)--; + } + } +#endif + inline void HOT write_footer_to_buffer_(char *buffer, uint16_t *buffer_at, uint16_t buffer_size) { static constexpr uint16_t RESET_COLOR_LEN = sizeof(ESPHOME_LOG_RESET_COLOR) - 1; this->write_body_to_buffer_(ESPHOME_LOG_RESET_COLOR, RESET_COLOR_LEN, buffer, buffer_at, buffer_size);