Skip to content

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 force-pushed the main branch 3 times, most recently from 2342fb4 to fb830a3 Compare September 15, 2024 16:23
@jjyyxx jjyyxx changed the title Fix fcntl_setpipe_size return value (#1163) Fix fcntl_setpipe_size return value Sep 15, 2024
@jjyyxx jjyyxx force-pushed the main branch 2 times, most recently from 397a089 to 6a80c79 Compare September 15, 2024 16:28
`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