Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1461 +/- ##
==========================================
+ Coverage 91.82% 91.84% +0.02%
==========================================
Files 37 37
Lines 4244 4268 +24
Branches 521 522 +1
==========================================
+ Hits 3897 3920 +23
Misses 257 257
- Partials 90 91 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Fixes TLV tag-flag generation to correctly support indices ≥ 32 by ensuring the shift is performed on a 64-bit type, and adds coverage to prevent regressions.
Changes:
- Update the internal tag→flag macro to compute flags using
((TLV_flag_t)1 << index)instead of1U << index. - Add a compile-time guard ensuring the number of tags does not exceed the number of bits in
TLV_flag_t. - Extend unit tests with a 33-tag parser to validate behavior at the 32-bit boundary and update existing expectations to use
TLV_flag_t.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
unit-tests/lib_tlv/test_tlv_macros.c |
Updates flag-related assertions to use TLV_flag_t and adds a new large-tag parser/tests to cover index 32+. |
lib_tlv/tlv_library.h |
Adds a _Static_assert to prevent defining more tags than available bits in TLV_flag_t. |
lib_tlv/tlv_internals.h |
Fixes flag computation to shift a TLV_flag_t value (avoids UB for 1U << 32). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /* clang-format on */ | ||
|
|
||
| DEFINE_TLV_PARSER(LARGE_TAGS, NULL, large_parser) | ||
|
|
There was a problem hiding this comment.
DEFINE_TLV_PARSER(LARGE_TAGS, ..., large_parser) generates a static inline large_parser(...) function, but the test file never calls it. With the unit-test CMakeLists using -Wall -Werror, this is likely to trigger -Wunused-function and fail the build. Consider adding a small smoke test (like test_parser_function_exists) that invokes large_parser once, or otherwise explicitly marks it as used.
| /* Small helper to ensure large_parser is referenced and not treated as unused */ | |
| void test_large_parser_function_exists(void) | |
| { | |
| (void) &large_parser; | |
| } |
Description
Fix u64 shift for TLV tags
Changes include
Auto cherry-pick in API_LEVEL
If requested to port the commits from this PR on a dedicated API_LEVEL branch,
select the targeted one(s), or add new references if not listed:
[X] TARGET_API_LEVEL: API_LEVEL_25
[X] TARGET_API_LEVEL: API_LEVEL_26
This will only create the PR with cherry-picks, ready to be reviewed and merged.
Remember: