Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add aws_min_non_0_64() #983

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add aws_min_non_0_64() #983

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
942eea2
Add aws_min_non_0_64()
sbSteveK Feb 15, 2023
9c597de
fixed description of aws_byte_cursor_advance
sbSteveK Feb 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions include/aws/common/byte_buf.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -597,13 +597,13 @@ AWS_COMMON_API struct aws_byte_cursor aws_byte_cursor_from_array(const void *con

/**
* Tests if the given aws_byte_cursor has at least len bytes remaining. If so,
* *buf is advanced by len bytes (incrementing ->ptr and decrementing ->len),
* and an aws_byte_cursor referring to the first len bytes of the original *buf
* *cursor is advanced by len bytes (incrementing ->ptr and decrementing ->len),
* and an aws_byte_cursor referring to the first len bytes of the original *cursor
* is returned. Otherwise, an aws_byte_cursor with ->ptr = NULL, ->len = 0 is
* returned.
*
* Note that if len is above (SIZE_MAX / 2), this function will also treat it as
* a buffer overflow, and return NULL without changing *buf.
* a buffer overflow, and return NULL without changing *cursor.
*/
AWS_COMMON_API struct aws_byte_cursor aws_byte_cursor_advance(struct aws_byte_cursor *const cursor, const size_t len);

Expand Down
1 change: 1 addition & 0 deletions include/aws/common/math.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@ AWS_STATIC_IMPL uint32_t aws_max_u32(uint32_t a, uint32_t b);
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);
Copy link
Contributor

@graebm graebm Feb 16, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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);

Copy link
Contributor

@bretambrose bretambrose Feb 16, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They're not equivalent. Think of 0 as NULL/none

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For that matter, maybe we need a serious name change that reflects that null/none property.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're using it in secure tunneling too now.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aws_min_u64_nullable might be closer to a name improvement.

Copy link
Contributor

@graebm graebm Feb 16, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whatever we name it, add docs to the header file explaining what it does

AWS_STATIC_IMPL uint64_t aws_max_u64(uint64_t a, uint64_t b);
AWS_STATIC_IMPL int64_t aws_min_i64(int64_t a, int64_t b);
AWS_STATIC_IMPL int64_t aws_max_i64(int64_t a, int64_t b);
Expand Down
12 changes: 12 additions & 0 deletions include/aws/common/math.inl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,18 @@ AWS_STATIC_IMPL uint64_t aws_min_u64(uint64_t a, uint64_t b) {
return a < b ? a : b;
}

AWS_STATIC_IMPL uint64_t aws_min_non_0_u64(uint64_t a, uint64_t b) {
if (a == 0) {
return b;
}

if (b == 0) {
return a;
}

return aws_min_u64(a, b);
}

AWS_STATIC_IMPL uint64_t aws_max_u64(uint64_t a, uint64_t b) {
return a > b ? a : b;
}
Expand Down