Skip to content

Commit

Permalink
refactor(ffi): return null ptr instead of aborting in C API (#2478)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
nylanderdev authored Mar 26, 2021
1 parent 68d4e4a commit 895e4cf
Show file tree
Hide file tree
Showing 7 changed files with 33 additions and 25 deletions.
10 changes: 5 additions & 5 deletions src/ffi/body.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ ffi_fn! {
/// If not configured, this body acts as an empty payload.
fn hyper_body_new() -> *mut hyper_body {
Box::into_raw(Box::new(hyper_body(Body::empty())))
}
} ?= ptr::null_mut()
}

ffi_fn! {
Expand Down Expand Up @@ -66,7 +66,7 @@ ffi_fn! {
Box::into_raw(hyper_task::boxed(async move {
body.0.data().await.map(|res| res.map(hyper_buf))
}))
}
} ?= ptr::null_mut()
}

ffi_fn! {
Expand Down Expand Up @@ -97,7 +97,7 @@ ffi_fn! {
}
Ok(())
}))
}
} ?= ptr::null_mut()
}

ffi_fn! {
Expand Down Expand Up @@ -198,7 +198,7 @@ ffi_fn! {
std::slice::from_raw_parts(buf, len)
};
Box::into_raw(Box::new(hyper_buf(Bytes::copy_from_slice(slice))))
}
} ?= ptr::null_mut()
}

ffi_fn! {
Expand All @@ -211,7 +211,7 @@ ffi_fn! {
/// consumed/freed.
fn hyper_buf_bytes(buf: *const hyper_buf) -> *const u8 {
unsafe { (*buf).0.as_ptr() }
}
} ?= ptr::null()
}

ffi_fn! {
Expand Down
6 changes: 3 additions & 3 deletions src/ffi/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ ffi_fn! {
hyper_clientconn { tx }
})
}))
}
} ?= std::ptr::null_mut()
}

ffi_fn! {
Expand Down Expand Up @@ -85,7 +85,7 @@ ffi_fn! {
};

Box::into_raw(hyper_task::boxed(fut))
}
} ?= std::ptr::null_mut()
}

ffi_fn! {
Expand All @@ -110,7 +110,7 @@ ffi_fn! {
builder: conn::Builder::new(),
exec: WeakExec::new(),
}))
}
} ?= std::ptr::null_mut()
}

ffi_fn! {
Expand Down
10 changes: 5 additions & 5 deletions src/ffi/http_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ ffi_fn! {
/// Construct a new HTTP request.
fn hyper_request_new() -> *mut hyper_request {
Box::into_raw(Box::new(hyper_request(Request::new(Body::empty()))))
}
} ?= std::ptr::null_mut()
}

ffi_fn! {
Expand Down Expand Up @@ -114,7 +114,7 @@ ffi_fn! {
/// `hyper_request` has been consumed.
fn hyper_request_headers(req: *mut hyper_request) -> *mut hyper_headers {
hyper_headers::get_or_default(unsafe { &mut *req }.0.extensions_mut())
}
} ?= std::ptr::null_mut()
}

ffi_fn! {
Expand Down Expand Up @@ -170,7 +170,7 @@ ffi_fn! {
/// buffer.
fn hyper_response_reason_phrase(resp: *const hyper_response) -> *const u8 {
unsafe { &*resp }.reason_phrase().as_ptr()
}
} ?= std::ptr::null()
}

ffi_fn! {
Expand Down Expand Up @@ -210,7 +210,7 @@ ffi_fn! {
/// `hyper_response` has been freed.
fn hyper_response_headers(resp: *mut hyper_response) -> *mut hyper_headers {
hyper_headers::get_or_default(unsafe { &mut *resp }.0.extensions_mut())
}
} ?= std::ptr::null_mut()
}

ffi_fn! {
Expand All @@ -220,7 +220,7 @@ ffi_fn! {
fn hyper_response_body(resp: *mut hyper_response) -> *mut hyper_body {
let body = std::mem::take(unsafe { &mut *resp }.0.body_mut());
Box::into_raw(Box::new(hyper_body(body)))
}
} ?= std::ptr::null_mut()
}

impl hyper_response {
Expand Down
2 changes: 1 addition & 1 deletion src/ffi/io.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ ffi_fn! {
write: write_noop,
userdata: std::ptr::null_mut(),
}))
}
} ?= std::ptr::null_mut()
}

ffi_fn! {
Expand Down
18 changes: 13 additions & 5 deletions src/ffi/macros.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
macro_rules! ffi_fn {
($(#[$doc:meta])* fn $name:ident($($arg:ident: $arg_ty:ty),*) -> $ret:ty $body:block) => {
($(#[$doc:meta])* fn $name:ident($($arg:ident: $arg_ty:ty),*) -> $ret:ty $body:block ?= $default:expr) => {
$(#[$doc])*
#[no_mangle]
pub extern fn $name($($arg: $arg_ty),*) -> $ret {
Expand All @@ -8,15 +8,23 @@ macro_rules! ffi_fn {
match panic::catch_unwind(AssertUnwindSafe(move || $body)) {
Ok(v) => v,
Err(_) => {
// TODO: We shouldn't abort, but rather figure out how to
// convert into the return type that the function errored.
eprintln!("panic unwind caught, aborting");
std::process::abort();
$default
}
}
}
};

($(#[$doc:meta])* fn $name:ident($($arg:ident: $arg_ty:ty),*) -> $ret:ty $body:block) => {
ffi_fn!($(#[$doc])* fn $name($($arg: $arg_ty),*) -> $ret $body ?= {
eprintln!("panic unwind caught, aborting");
std::process::abort()
});
};

($(#[$doc:meta])* fn $name:ident($($arg:ident: $arg_ty:ty),*) $body:block ?= $default:expr) => {
ffi_fn!($(#[$doc])* fn $name($($arg: $arg_ty),*) -> () $body ?= $default);
};

($(#[$doc:meta])* fn $name:ident($($arg:ident: $arg_ty:ty),*) $body:block) => {
ffi_fn!($(#[$doc])* fn $name($($arg: $arg_ty),*) -> () $body);
};
Expand Down
2 changes: 1 addition & 1 deletion src/ffi/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,5 +92,5 @@ ffi_fn! {
/// Returns a static ASCII (null terminated) string of the hyper version.
fn hyper_version() -> *const libc::c_char {
VERSION_CSTR.as_ptr() as _
}
} ?= std::ptr::null()
}
10 changes: 5 additions & 5 deletions src/ffi/task.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ ffi_fn! {
/// Creates a new task executor.
fn hyper_executor_new() -> *const hyper_executor {
Arc::into_raw(hyper_executor::new())
}
} ?= ptr::null()
}

ffi_fn! {
Expand Down Expand Up @@ -230,7 +230,7 @@ ffi_fn! {
Some(task) => Box::into_raw(task),
None => ptr::null_mut(),
}
}
} ?= ptr::null_mut()
}

// ===== impl hyper_task =====
Expand Down Expand Up @@ -303,7 +303,7 @@ ffi_fn! {
} else {
ptr::null_mut()
}
}
} ?= ptr::null_mut()
}

ffi_fn! {
Expand Down Expand Up @@ -341,7 +341,7 @@ ffi_fn! {
}

unsafe { &*task }.userdata.0
}
} ?= ptr::null_mut()
}

// ===== impl AsTaskType =====
Expand Down Expand Up @@ -405,7 +405,7 @@ ffi_fn! {
fn hyper_context_waker(cx: *mut hyper_context<'_>) -> *mut hyper_waker {
let waker = unsafe { &mut *cx }.0.waker().clone();
Box::into_raw(Box::new(hyper_waker { waker }))
}
} ?= ptr::null_mut()
}

// ===== impl hyper_waker =====
Expand Down

0 comments on commit 895e4cf

Please sign in to comment.