From ef52ce4d76e5a5c5d469b64b79bd7d83a15b349c Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 18 Oct 2025 11:56:40 -1000 Subject: [PATCH] [api_protobuf] Address copilot review: add bounds checking and clarify 32-bit loop intent - Add bounds checking in decode_varint_content to prevent undefined behavior if decoded enum value exceeds 31 - Add clarifying comments that 32-bit loops in encode_content and get_size_calculation are intentional to support the full range of enum_as_bitmask (enums with up to 32 values) - The uint32_t storage type supports general-purpose enum_as_bitmask, not just ColorMode's 10 values --- script/api_protobuf/api_protobuf.py | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/script/api_protobuf/api_protobuf.py b/script/api_protobuf/api_protobuf.py index 2fe6d01024..f423097b7f 100755 --- a/script/api_protobuf/api_protobuf.py +++ b/script/api_protobuf/api_protobuf.py @@ -1472,11 +1472,16 @@ class RepeatedTypeInfo(TypeInfo): if self._use_pointer: return None if self._use_bitmask: - # For bitmask fields, decode enum value and set corresponding bit + # For bitmask fields, decode enum value and set corresponding bit, with bounds checking content = self._ti.decode_varint if content is None: return None - return f"case {self.number}: this->{self.field_name} |= (1U << static_cast({content})); break;" + return ( + f"case {self.number}: " + f"if (static_cast({content}) <= 31) " + f"this->{self.field_name} |= (1U << static_cast({content})); " + f"break;" + ) content = self._ti.decode_varint if content is None: return None @@ -1533,6 +1538,9 @@ class RepeatedTypeInfo(TypeInfo): if self._use_bitmask: # For bitmask fields, iterate through set bits and encode each enum value # The bitmask is stored as uint32_t where bit N represents enum value N + # Note: We iterate through all 32 bits to support the full range of enum_as_bitmask + # (enums with up to 32 values). Specific uses may have fewer values, but the + # generated code is general-purpose. assert isinstance(self._ti, EnumType), ( "enum_as_bitmask only works with enum fields" ) @@ -1587,6 +1595,9 @@ class RepeatedTypeInfo(TypeInfo): if self._use_bitmask: # For bitmask fields, iterate through set bits and calculate size # Each set bit encodes one enum value (as varint) + # Note: We iterate through all 32 bits to support the full range of enum_as_bitmask + # (enums with up to 32 values). Specific uses may have fewer values, but the + # generated code is general-purpose. o = f"if ({name} != 0) {{\n" o += " for (uint8_t bit = 0; bit < 32; bit++) {\n" o += f" if ({name} & (1U << bit)) {{\n"