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

Fix fcntl_setpipe_size return value #1164

Merged
merged 1 commit into from
Sep 16, 2024
Merged

Conversation

jjyyxx
Copy link
Contributor

@jjyyxx jjyyxx commented Sep 15, 2024

fcntl man page says that:

F_SETPIPE_SZ ... The actual capacity (in bytes) that is set is
returned as the function result.

But the current fcntl_setpipe_size function assumes that the return value is 0 (success) or negative (failure). This makes the function panic due to debug_assert! in debug mode, and it returns Err on success in release mode.

This commit fixes the return value type and its checking, and adds a test for it.

Closes #1163

@jjyyxx jjyyxx changed the title Fix fcntl_setpipe_size return value (#1163) Fix fcntl_setpipe_size return value Sep 15, 2024
`fcntl` man page says that:
> F_SETPIPE_SZ ... The actual capacity (in bytes) that is set is
> returned as the function result.

But the current `fcntl_setpipe_size` function assumes that the return
value is 0 (success) or negative (failure). This makes the function
panic due to `debug_assert!` in debug mode, and it returns `Err` on
success in release mode.

This commit fixes the return value type and its checking, and adds a
test for it.
@sunfishcode
Copy link
Member

Thanks!

@sunfishcode sunfishcode merged commit c8fe26b into bytecodealliance:main Sep 16, 2024
42 of 43 checks passed
sunfishcode added a commit that referenced this pull request Sep 16, 2024
Backport parts of #1164 and #1165 to 0.38, but without the change to the
public return type, to preserve semver compatibility for 0.38.
sunfishcode added a commit that referenced this pull request Sep 17, 2024
Backport parts of #1164 and #1165 to 0.38, but without the change to the
public return type, to preserve semver compatibility for 0.38.
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.

fcntl_setpipe_size returns pipe size on success, not 0
2 participants