Add aws_min_non_0_64() - #983
Conversation
Codecov ReportBase: 80.87% // Head: 80.78% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #983 +/- ##
==========================================
- Coverage 80.87% 80.78% -0.09%
==========================================
Files 53 53
Lines 5678 5678
==========================================
- Hits 4592 4587 -5
- Misses 1086 1091 +5
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
| AWS_STATIC_IMPL int32_t aws_min_i32(int32_t a, int32_t b); | ||
| AWS_STATIC_IMPL int32_t aws_max_i32(int32_t a, int32_t b); | ||
| AWS_STATIC_IMPL uint64_t aws_min_u64(uint64_t a, uint64_t b); | ||
| AWS_STATIC_IMPL uint64_t aws_min_non_0_u64(uint64_t a, uint64_t b); |
There was a problem hiding this comment.
something like clamp() seems more generically useful?
uint64_t aws_clamp_u64(uint64_t val, uint64_t low, uint64_t high);
using it would look like:
next_service_time = aws_clamp_u64(operation_processing_time, 1, next_service_time);
which is equivalent to the proposed:
next_service_time = aws_min_non_0_64(operation_processing_time, next_service_time);
There was a problem hiding this comment.
They're not equivalent. Think of 0 as NULL/none
There was a problem hiding this comment.
For that matter, maybe we need a serious name change that reflects that null/none property.
There was a problem hiding this comment.
This all comes from the mqtt5 client architecture for composing next service time calculations. 0 means no-affect, non-zero means a time constraint. We want the smallest non-zero time constraint as our next scheduled time.
There was a problem hiding this comment.
maybe this should just stay an MQTT5 helper function, it's kinda weird as a generic math operation.
Looking at this header file, which is just a function name with no docs, assumed it meant "min, but higher than 0".
There was a problem hiding this comment.
We're using it in secure tunneling too now.
There was a problem hiding this comment.
aws_min_u64_nullable might be closer to a name improvement.
There was a problem hiding this comment.
whatever we name it, add docs to the header file explaining what it does
helper function aws_min_non_0_64 used in multiple places moved to common.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.