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

Return null instead of aborting in C API by extending ffi_fn! macro #2478

Merged
merged 1 commit into from Mar 26, 2021

Conversation

nylanderdev
Copy link
Contributor

@nylanderdev nylanderdev commented Mar 24, 2021

Make C API functions that return pointers return null in case of a
panic, instead of aborting.
Add ffi_fn! macro rules that enable error values to be returned by
writing "?= " or "?: " after an ffi function's body.

Incremental to #2397

Copy link
Member

@seanmonstar seanmonstar left a comment

Choose a reason for hiding this comment

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

Awesome start! I left some thoughts inline.

src/ffi/macros.rs Outdated Show resolved Hide resolved
src/ffi/macros.rs Show resolved Hide resolved
src/ffi/macros.rs Outdated Show resolved Hide resolved
@nylanderdev
Copy link
Contributor Author

nylanderdev commented Mar 25, 2021

How do we want to deal with functions that return ()? Right now they don't support default values (since the only valid one is ()) but that also makes them default to aborting. We could either add ?= support to them, and write ?= (), or we could use some shorthand, like just writing ? to mark those that won't abort. We could also make not aborting the default for ()-functions, and add a ! or a !! to explicitly signify an aborting function.

@seanmonstar
Copy link
Member

Can we mark them as ?= ()? I'd rather the explicitness than accidentally forgetting whether the value should have returned an error if it panicked. I assume that those that don't return a value, in practice wouldn't ever fail or panic, but we catch anyways just in case.

@nylanderdev nylanderdev force-pushed the dont-panic branch 2 times, most recently from 1b5c59c to 0dddb29 Compare March 25, 2021 23:46
Make C API functions that return pointers return null in case of a
panic, instead of aborting.
Add ffi_fn! macro rules that enable default error values to be returned
by writing "?= <value>" after an ffi function's body.
@nylanderdev
Copy link
Contributor Author

nylanderdev commented Mar 26, 2021

Supports ?= () now. Have not marked any more functions however since I don't know which we want to catch on. If all ()-functions should be caught then I've got a commit on standby.

Copy link
Member

@seanmonstar seanmonstar left a comment

Choose a reason for hiding this comment

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

Awesome work, thanks for starting this! I think we can do follow up PRs to add this to more of the functions (and then eventually remove the ability to have forgotten).

@seanmonstar seanmonstar merged commit 895e4cf into hyperium:master Mar 26, 2021
@nylanderdev nylanderdev deleted the dont-panic branch April 27, 2021 15:59
BenxiangGe pushed a commit to BenxiangGe/hyper that referenced this pull request Jul 26, 2021
…#2478)

Make C API functions that return pointers return null in case of a
panic, instead of aborting.

Add ffi_fn! macro rules that enable default error values to be returned
by writing "?= <value>" after an ffi function's body.
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.

None yet

2 participants