Address Copilot review: uint32 overflow guard, docstring, test namespace/include

This commit is contained in:
J. Nick Koston
2026-04-14 08:36:09 -10:00
parent 57d9e508ea
commit 5066171a9d
3 changed files with 19 additions and 9 deletions
+12 -6
View File
@@ -545,17 +545,19 @@ std::string value_accuracy_to_string(float value, int8_t accuracy_decimals) {
// Fast float-to-string for accuracy_decimals 0-3 (covers virtually all sensor usage).
// Avoids snprintf("%.*f") which pulls in heavy float formatting machinery.
static size_t value_accuracy_to_buf_fast(char *buf, float value, int8_t accuracy_decimals) {
// Caller must guarantee value is finite and |value| * mult fits in uint32_t.
static size_t value_accuracy_to_buf_fast(char *buf, float value, int8_t accuracy_decimals, uint32_t mult) {
char *p = buf;
if (std::signbit(value)) {
*p++ = '-';
value = -value;
}
uint32_t mult = small_pow10(accuracy_decimals);
// Cast to double for the multiply to match snprintf's rounding precision.
// float*int loses bits at exact-half boundaries (e.g. 23.45f*10 = 234.5 in float,
// but snprintf sees 234.500007... via double promotion and rounds differently).
uint32_t scaled = static_cast<uint32_t>(lrint(static_cast<double>(value) * mult));
// llrint returns long long so the result fits even on 32-bit targets where
// long is 32-bit; caller has already bounded |value * mult| to UINT32_MAX.
uint32_t scaled = static_cast<uint32_t>(llrint(static_cast<double>(value) * mult));
p = uint32_to_str_unchecked(p, scaled / mult);
if (accuracy_decimals > 0) {
*p++ = '.';
@@ -568,12 +570,16 @@ static size_t value_accuracy_to_buf_fast(char *buf, float value, int8_t accuracy
size_t value_accuracy_to_buf(std::span<char, VALUE_ACCURACY_MAX_LEN> buf, float value, int8_t accuracy_decimals) {
normalize_accuracy_decimals(value, accuracy_decimals);
// Fast path for accuracy 0-3 and finite values
// Fast path for accuracy 0-3, finite values whose scaled magnitude fits in uint32_t.
// For 3 decimals that's |value| < ~4.29e6; larger totals fall through to snprintf.
if (accuracy_decimals <= 3 && std::isfinite(value)) {
return value_accuracy_to_buf_fast(buf.data(), value, accuracy_decimals);
const uint32_t mult = small_pow10(accuracy_decimals);
if (std::fabs(value) < static_cast<float>(UINT32_MAX) / mult) {
return value_accuracy_to_buf_fast(buf.data(), value, accuracy_decimals, mult);
}
}
// Fallback for NaN/Inf/high accuracy
// Fallback for NaN/Inf/high accuracy/out-of-range
int len = snprintf(buf.data(), buf.size(), "%.*f", accuracy_decimals, value);
if (len < 0)
return 0;
+4 -1
View File
@@ -1296,7 +1296,10 @@ inline char *int8_to_str(char *buf, int8_t val) {
}
/// Append a separator char and a string to a buffer, respecting remaining space.
/// Returns pointer past last char written (null terminator is written).
/// Returns pointer past last char written. On success (remaining >= 2) a null
/// terminator is written after the copied string. If remaining < 2 nothing is
/// written (not even a terminator) and `buf` is returned unchanged — callers
/// needing a terminated buffer in that case must ensure one is already present.
inline char *buf_append_sep_str(char *buf, size_t remaining, char separator, const char *str, size_t str_len) {
if (remaining < 2) {
return buf;
@@ -3,10 +3,11 @@
#include <cstdio>
#include <cstring>
#include <span>
#include <string>
#include "esphome/core/helpers.h"
namespace esphome::testing {
namespace esphome::core::testing {
// Helper to call value_accuracy_to_buf and return as string
static std::string va_to_string(float value, int8_t accuracy_decimals) {
@@ -148,4 +149,4 @@ TEST(ValueAccuracyToBuf, ReturnsCorrectLength) {
EXPECT_EQ(strlen(buf), len);
}
} // namespace esphome::testing
} // namespace esphome::core::testing