Skip to content

shared: Introduce max_message_size_… features#331

Merged
geonnave merged 10 commits intolake-rs:mainfrom
chrysn-pull-requests:size-granularity
Feb 3, 2025
Merged

shared: Introduce max_message_size_… features#331
geonnave merged 10 commits intolake-rs:mainfrom
chrysn-pull-requests:size-granularity

Conversation

@chrysn
Copy link
Copy Markdown
Member

@chrysn chrysn commented Jan 16, 2025

Closes: #330

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

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

@chrysn chrysn mentioned this pull request Jan 22, 2025
@chrysn
Copy link
Copy Markdown
Member 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
Copy Markdown
Member 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
Copy Markdown
Collaborator

geonnave commented Feb 3, 2025

Merging!!

@geonnave geonnave merged commit 0ba2ca4 into lake-rs:main Feb 3, 2025
@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