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.

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

shared: Introduce max_message_size_… features #331

Merged
merged 10 commits into from
Feb 3, 2025

Conversation

chrysn
Copy link
Collaborator

@chrysn chrysn commented Jan 16, 2025

Closes: #330

@chrysn chrysn requested a review from geonnave January 16, 2025 15:27
@geonnave
Copy link
Collaborator

Thank you for the PR. Looking into why CI fails.

@chrysn chrysn mentioned this pull request Jan 22, 2025
@chrysn
Copy link
Collaborator Author

chrysn commented Feb 3, 2025

Unlike on #333 I did not rebase this after #339 to fix CI, but instead merged main back: This enables this PR to stay on 0.7.2, i.e. to be a single fixing change on the latest release (which is how I use it on Ariel OS).

@geonnave, is there anything more you'd like to review? Can we merge this as is, or should we trick around (possibly by briefly disabling CI checks) to preserve both a "feature branch simply merges back" shape and the "this can be picked into a 0.7 branch" property? (I think it's fine to merge).

@chrysn
Copy link
Collaborator Author

chrysn commented Feb 3, 2025

Aaaargh: cbindgen doesn't support switching by cfg (while it supported the old idiom of cfg'd items). [edit:] Using cfg-if doesn't help either.

Emulating the old idiom doesn't scale well (the entry for the shortest size would need to be not(any(feature = "first", feature = "second", ...))), I'll look for alternatives.

(ceterum censeo: exposing the API to C is a liability for this project)

With the recently increased buffer sizes, the original test failed to
achieve its goal of producing an overflow error.
When this test failed, the resulting "Type state mismatch" runtime error
was confusing -- and while currently we raise the buffer error first,
there's no guarantee that that couldn't flip around. Therefore, the
example is made more realistic to ensure that it's really the buffer
error that triggers first.
@chrysn chrysn force-pushed the size-granularity branch 2 times, most recently from 0268159 to c024fce Compare February 3, 2025 12:14
@geonnave
Copy link
Collaborator

geonnave commented Feb 3, 2025

Merging!!

@geonnave geonnave merged commit 0ba2ca4 into openwsn-berkeley:main Feb 3, 2025
37 checks passed
@chrysn chrysn deleted the size-granularity branch February 3, 2025 19:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MAX_MESSAGE_SIZE configuration granularity is insufficient
2 participants