From 5be5f1666222a622355e23de4a48b2af7692c02f Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 26 Mar 2026 13:52:41 -1000 Subject: [PATCH] Fix compat: heap-allocate deprecated vector, add find fallback Two bugs fixed from Copilot review: 1. Dangling pointer on copy: deprecated setters stored data in an OwnedPresetModes struct on FanTraits. If the traits object was copied, the copy's pointer dangled. Fix: deprecated setters now heap-allocate (intentional leak). Pointer survives any copy. Remove the OwnedPresetModes wrapper entirely. 2. find_preset_mode_ / save_state_ only searched the Fan-owned vector, breaking external components using the deprecated traits setters. Fix: fall back to get_traits() when the entity vector is null. --- esphome/components/fan/fan.cpp | 26 ++++++++++++++++--------- esphome/components/fan/fan_traits.h | 30 ++++++++++------------------- 2 files changed, 27 insertions(+), 29 deletions(-) diff --git a/esphome/components/fan/fan.cpp b/esphome/components/fan/fan.cpp index f486c40ca44..3f4b7844c0f 100644 --- a/esphome/components/fan/fan.cpp +++ b/esphome/components/fan/fan.cpp @@ -148,13 +148,19 @@ const char *Fan::find_preset_mode_(const char *preset_mode) { } const char *Fan::find_preset_mode_(const char *preset_mode, size_t len) { - if (preset_mode == nullptr || len == 0 || !this->supported_preset_modes_) + if (preset_mode == nullptr || len == 0) { return nullptr; - for (const char *mode : *this->supported_preset_modes_) { - if (strncmp(mode, preset_mode, len) == 0 && mode[len] == '\0') - return mode; } - return nullptr; + if (this->supported_preset_modes_) { + for (const char *mode : *this->supported_preset_modes_) { + if (strncmp(mode, preset_mode, len) == 0 && mode[len] == '\0') { + return mode; + } + } + return nullptr; + } + // Fallback for deprecated path: external components may set modes on FanTraits directly + return this->get_traits().find_preset_mode(preset_mode, len); } bool Fan::set_preset_mode_(const char *preset_mode, size_t len) { @@ -274,10 +280,12 @@ void Fan::save_state_() { state.direction = this->direction; state.preset_mode = FanRestoreState::NO_PRESET; - if (this->has_preset_mode() && this->supported_preset_modes_) { - // Find index of current preset mode (pointer comparison is safe since preset is from our vector) - for (size_t i = 0; i < this->supported_preset_modes_->size(); i++) { - if ((*this->supported_preset_modes_)[i] == this->preset_mode_) { + if (this->has_preset_mode()) { + // Use Fan-owned vector, or fall back to traits for deprecated path + const auto &preset_modes = + this->supported_preset_modes_ ? *this->supported_preset_modes_ : this->get_traits().supported_preset_modes(); + for (size_t i = 0; i < preset_modes.size(); i++) { + if (preset_modes[i] == this->preset_mode_) { state.preset_mode = i; break; } diff --git a/esphome/components/fan/fan_traits.h b/esphome/components/fan/fan_traits.h index b5d6556e3bc..b6549368c9d 100644 --- a/esphome/components/fan/fan_traits.h +++ b/esphome/components/fan/fan_traits.h @@ -33,7 +33,8 @@ class FanTraits { void set_direction(bool direction) { this->direction_ = direction; } /// Return the preset modes supported by the fan. const std::vector &supported_preset_modes() const { - return this->preset_modes_ ? *this->preset_modes_ : this->owned_preset_modes_.modes; + static const std::vector EMPTY_VECTOR; + return this->preset_modes_ ? *this->preset_modes_ : EMPTY_VECTOR; } /// Set the preset modes pointer (points to vector owned by Fan base class). void set_supported_preset_modes(const std::vector *preset_modes) { this->preset_modes_ = preset_modes; } @@ -41,20 +42,17 @@ class FanTraits { // Remove before 2026.11.0 ESPDEPRECATED("Call set_supported_preset_modes() on the Fan entity instead. Removed in 2026.11.0", "2026.5.0") void set_supported_preset_modes(std::initializer_list preset_modes) { - this->owned_preset_modes_.modes = preset_modes; - this->preset_modes_ = &this->owned_preset_modes_.modes; + // NOLINT - intentional leak: pointer must survive copies of FanTraits + this->preset_modes_ = new std::vector(preset_modes); // NOLINT } // Remove before 2026.11.0 ESPDEPRECATED("Call set_supported_preset_modes() on the Fan entity instead. Removed in 2026.11.0", "2026.5.0") void set_supported_preset_modes(const std::vector &preset_modes) { - this->owned_preset_modes_.modes = preset_modes; - this->preset_modes_ = &this->owned_preset_modes_.modes; + this->preset_modes_ = new std::vector(preset_modes); // NOLINT } /// Return if preset modes are supported - bool supports_preset_modes() const { - return (this->preset_modes_ && !this->preset_modes_->empty()) || !this->owned_preset_modes_.modes.empty(); - } + bool supports_preset_modes() const { return this->preset_modes_ && !this->preset_modes_->empty(); } /// Find and return the matching preset mode pointer from supported modes, or nullptr if not found. const char *find_preset_mode(const char *preset_mode) const { return this->find_preset_mode(preset_mode, preset_mode ? strlen(preset_mode) : 0); @@ -62,7 +60,10 @@ class FanTraits { const char *find_preset_mode(const char *preset_mode, size_t len) const { if (preset_mode == nullptr || len == 0) return nullptr; - const auto &modes = this->preset_modes_ ? *this->preset_modes_ : this->owned_preset_modes_.modes; + if (!this->preset_modes_) { + return nullptr; + } + const auto &modes = *this->preset_modes_; for (const char *mode : modes) { if (strncmp(mode, preset_mode, len) == 0 && mode[len] == '\0') { return mode; // Return pointer from traits @@ -77,17 +78,6 @@ class FanTraits { bool direction_{false}; int speed_count_{}; const std::vector *preset_modes_{nullptr}; - /** Compat storage for deprecated setters — skipped on copy to avoid overhead. - * Remove in 2026.11.0 along with the deprecated overloads. - */ - struct OwnedPresetModes { - std::vector modes; - OwnedPresetModes() = default; - OwnedPresetModes(const OwnedPresetModes &) {} // NOLINT - no-op copy: compat data is not propagated - OwnedPresetModes &operator=(const OwnedPresetModes &) { return *this; } // NOLINT - OwnedPresetModes(OwnedPresetModes &&) = default; - OwnedPresetModes &operator=(OwnedPresetModes &&) = default; - } owned_preset_modes_; }; } // namespace fan