From 0c74b569198fe6283ddb9a18e329f8112d1d5c7f Mon Sep 17 00:00:00 2001 From: Justus Winter Date: Wed, 9 Jan 2019 12:20:16 +0100 Subject: ffi: Introduce macro for moving ownership from C. --- ffi/src/core.rs | 5 ++-- ffi/src/lib.rs | 16 ++++++++++ ffi/src/openpgp/armor.rs | 10 ++----- ffi/src/openpgp/mod.rs | 77 +++++++++++------------------------------------- ffi/src/openpgp/tpk.rs | 48 +++++++++--------------------- ffi/src/store.rs | 14 ++------- 6 files changed, 54 insertions(+), 116 deletions(-) diff --git a/ffi/src/core.rs b/ffi/src/core.rs index 564f3ae8..b207bdb9 100644 --- a/ffi/src/core.rs +++ b/ffi/src/core.rs @@ -181,11 +181,10 @@ pub extern "system" fn sq_context_ephemeral(ctx: Option<&Context>) -> uint8_t { /// Consumes `cfg`. Returns `NULL` on errors. Returns `NULL` on /// errors. If `errp` is not `NULL`, the error is stored there. #[no_mangle] -pub extern "system" fn sq_config_build(cfg: Option<&mut Config>, +pub extern "system" fn sq_config_build(cfg: *mut Config, errp: Option<&mut *mut failure::Error>) -> *mut Context { - assert!(cfg.is_some()); - let cfg = unsafe { Box::from_raw(cfg.unwrap()) }; + let cfg = ffi_param_move!(cfg); match cfg.build() { Ok(context) => diff --git a/ffi/src/lib.rs b/ffi/src/lib.rs index 3dd0a252..5c813a37 100644 --- a/ffi/src/lib.rs +++ b/ffi/src/lib.rs @@ -143,6 +143,22 @@ macro_rules! ffi_free { /* Parameter handling. */ +/// Transfers ownership from C to Rust. +/// +/// # Panics +/// +/// Panics if called with NULL. +macro_rules! ffi_param_move { + ($name:expr) => {{ + if $name.is_null() { + panic!("Parameter {} is NULL", stringify!($name)); + } + unsafe { + Box::from_raw($name) + } + }}; +} + /// Transfers a reference from C to Rust. /// /// # Panics diff --git a/ffi/src/openpgp/armor.rs b/ffi/src/openpgp/armor.rs index 9e6b3611..b3ed8edb 100644 --- a/ffi/src/openpgp/armor.rs +++ b/ffi/src/openpgp/armor.rs @@ -189,10 +189,7 @@ pub extern "system" fn sq_armor_reader_kind(reader: *mut Box) // little dance. We will momentarily take ownership of `reader`, // wrapping it in a Box again. Then, at the end of the function, // we will leak it again. - assert!(! reader.is_null()); - let reader = unsafe { - Box::from_raw(reader as *mut Box) - }; + let reader = ffi_param_move!(reader as *mut Box); let kind = kind_to_int(reader.kind()); Box::into_raw(reader); kind @@ -221,16 +218,13 @@ pub extern "system" fn sq_armor_reader_headers(ctx: Option<&mut Context>, len: Option<&mut size_t>) -> *mut ArmorHeader { let ctx = ffi_param_ref!(ctx); - assert!(! reader.is_null()); let len = ffi_param_ref!(len); // We need to downcast `reader`. To do that, we need to do a // little dance. We will momentarily take ownership of `reader`, // wrapping it in a Box again. Then, at the end of the function, // we will leak it again. - let mut reader = unsafe { - Box::from_raw(reader as *mut Box) - }; + let mut reader = ffi_param_move!(reader as *mut Box); // We need to be extra careful here in order not to keep ownership // of `reader` in case of errors. diff --git a/ffi/src/openpgp/mod.rs b/ffi/src/openpgp/mod.rs index cef143af..8123c4fc 100644 --- a/ffi/src/openpgp/mod.rs +++ b/ffi/src/openpgp/mod.rs @@ -108,10 +108,7 @@ pub extern "system" fn sq_revocation_status_variant( rs: *mut RevocationStatus) -> c_int { - assert!(! rs.is_null()); - let rs = unsafe { - Box::from_raw(rs as *mut RevocationStatus) - }; + let rs = ffi_param_move!(rs as *mut RevocationStatus); let variant = revocation_status_to_int(rs.as_ref()); Box::into_raw(rs); variant @@ -170,9 +167,7 @@ pub extern "system" fn sq_tsk_tpk(tsk: Option<&TSK>) #[no_mangle] pub extern "system" fn sq_tsk_into_tpk(tsk: *mut TSK) -> *mut TPK { - let tsk = unsafe { - Box::from_raw(tsk) - }; + let tsk = ffi_param_move!(tsk); box_raw!(tsk.into_tpk()) } @@ -239,10 +234,8 @@ pub extern "system" fn sq_signature_free(s: *mut Signature) { pub extern "system" fn sq_signature_to_packet(s: *mut Signature) -> *mut Packet { - assert!(! s.is_null()); - unsafe { - box_raw!(Box::from_raw(s).to_packet()) - } + let s = ffi_param_move!(s); + box_raw!(s.to_packet()) } /// Returns the value of the `Signature` packet's Issuer subpacket. @@ -828,10 +821,7 @@ pub extern "system" fn sq_packet_parser_next<'a> ppr: Option<&mut *mut PacketParserResult<'a>>) -> Status { let ctx = ffi_param_ref!(ctx); - assert!(! pp.is_null()); - let pp = unsafe { - Box::from_raw(pp) - }; + let pp = ffi_param_move!(pp); match pp.next() { Ok((old_p, new_ppr)) => { @@ -875,10 +865,7 @@ pub extern "system" fn sq_packet_parser_recurse<'a> ppr: Option<&mut *mut PacketParserResult<'a>>) -> Status { let ctx = ffi_param_ref!(ctx); - assert!(! pp.is_null()); - let pp = unsafe { - Box::from_raw(pp) - }; + let pp = ffi_param_move!(pp); match pp.recurse() { Ok((old_p, new_ppr)) => { @@ -1008,10 +995,7 @@ pub extern "system" fn sq_packet_parser_result_packet_parser<'a> (ppr: *mut PacketParserResult<'a>) -> *mut PacketParser<'a> { - assert!(! ppr.is_null()); - let ppr = unsafe { - Box::from_raw(ppr) - }; + let ppr = ffi_param_move!(ppr); match *ppr { PacketParserResult::Some(pp) => box_raw!(pp), @@ -1039,10 +1023,7 @@ pub extern "system" fn sq_packet_parser_result_eof<'a> (ppr: *mut PacketParserResult<'a>) -> *mut PacketParserEOF { - assert!(! ppr.is_null()); - let ppr = unsafe { - Box::from_raw(ppr) - }; + let ppr = ffi_param_move!(ppr); match *ppr { PacketParserResult::Some(_) => { @@ -1073,10 +1054,7 @@ pub extern "system" fn sq_writer_stack_message (writer: *mut Box) -> *mut writer::Stack<'static, Cookie> { - assert!(!writer.is_null()); - let writer = unsafe { - Box::from_raw(writer) - }; + let writer = ffi_param_move!(writer); box_raw!(Message::new(writer)) } @@ -1127,9 +1105,7 @@ pub extern "system" fn sq_writer_stack_finalize_one { let ctx = ffi_param_ref!(ctx); if !writer.is_null() { - let writer = unsafe { - Box::from_raw(writer) - }; + let writer = ffi_param_move!(writer); maybe_box_raw!(fry!(ctx, writer.finalize_one())) } else { ptr::null_mut() @@ -1145,9 +1121,7 @@ pub extern "system" fn sq_writer_stack_finalize { let ctx = ffi_param_ref!(ctx); if !writer.is_null() { - let writer = unsafe { - Box::from_raw(writer) - }; + let writer = ffi_param_move!(writer); fry_status!(ctx, writer.finalize()) } else { Status::Success @@ -1167,10 +1141,7 @@ pub extern "system" fn sq_arbitrary_writer_new -> *mut writer::Stack<'static, Cookie> { let ctx = ffi_param_ref!(ctx); - assert!(!inner.is_null()); - let inner = unsafe { - Box::from_raw(inner) - }; + let inner = ffi_param_move!(inner); fry_box!(ctx, ArbitraryWriter::new(*inner, tag.into())) } @@ -1187,10 +1158,7 @@ pub extern "system" fn sq_signer_new -> *mut writer::Stack<'static, Cookie> { let ctx = ffi_param_ref!(ctx); - assert!(!inner.is_null()); - let inner = unsafe { - Box::from_raw(inner) - }; + let inner = ffi_param_move!(inner); let signers = ffi_param_ref!(signers); let signers = unsafe { slice::from_raw_parts(signers, signers_len) @@ -1207,10 +1175,7 @@ pub extern "system" fn sq_signer_new_detached -> *mut writer::Stack<'static, Cookie> { let ctx = ffi_param_ref!(ctx); - assert!(!inner.is_null()); - let inner = unsafe { - Box::from_raw(inner) - }; + let inner = ffi_param_move!(inner); let signers = signers.expect("Signers is NULL"); let signers = unsafe { slice::from_raw_parts(signers, signers_len) @@ -1229,10 +1194,7 @@ pub extern "system" fn sq_literal_writer_new -> *mut writer::Stack<'static, Cookie> { let ctx = ffi_param_ref!(ctx); - assert!(!inner.is_null()); - let inner = unsafe { - Box::from_raw(inner) - }; + let inner = ffi_param_move!(inner); fry_box!(ctx, LiteralWriter::new(*inner, DataFormat::Binary, None, @@ -1257,10 +1219,7 @@ pub extern "system" fn sq_encryptor_new -> *mut writer::Stack<'static, Cookie> { let ctx = ffi_param_ref!(ctx); - assert!(!inner.is_null()); - let inner = unsafe { - Box::from_raw(inner) - }; + let inner = ffi_param_move!(inner); let mut passwords_ = Vec::new(); if passwords_len > 0 { let passwords = passwords.expect("Passwords is NULL"); @@ -1646,9 +1605,7 @@ impl DecryptionHelper for DHelper { "Callback did not return a session key".into()).into()); } - let secret = unsafe { - Box::from_raw(secret) - }; + let secret = ffi_param_move!(secret); Ok(Some(*secret)) } diff --git a/ffi/src/openpgp/tpk.rs b/ffi/src/openpgp/tpk.rs index 23a11b22..814bac99 100644 --- a/ffi/src/openpgp/tpk.rs +++ b/ffi/src/openpgp/tpk.rs @@ -68,8 +68,7 @@ pub extern "system" fn sq_tpk_from_packet_pile(ctx: Option<&mut Context>, m: *mut PacketPile) -> *mut TPK { let ctx = ffi_param_ref!(ctx); - assert!(! m.is_null()); - let m = unsafe { Box::from_raw(m) }; + let m = ffi_param_move!(m); fry_box!(ctx, TPK::from_packet_pile(*m)) } @@ -98,8 +97,7 @@ pub extern "system" fn sq_tpk_from_packet_parser(ctx: Option<&mut Context>, -> *mut TPK { let ctx = ffi_param_ref!(ctx); - assert!(! ppr.is_null()); - let ppr = unsafe { Box::from_raw(ppr) }; + let ppr = ffi_param_move!(ppr); fry_box!(ctx, TPK::from_packet_parser(*ppr)) } @@ -152,10 +150,8 @@ pub extern "system" fn sq_tpk_merge(ctx: Option<&mut Context>, other: *mut TPK) -> *mut TPK { let ctx = ffi_param_ref!(ctx); - assert!(! tpk.is_null()); - let tpk = unsafe { Box::from_raw(tpk) }; - assert!(! other.is_null()); - let other = unsafe { Box::from_raw(other) }; + let tpk = ffi_param_move!(tpk); + let other = ffi_param_move!(other); fry_box!(ctx, tpk.merge(*other)) } @@ -173,8 +169,7 @@ pub extern "system" fn sq_tpk_merge_packets(ctx: Option<&mut Context>, packets_len: size_t) -> *mut TPK { let ctx = ffi_param_ref!(ctx); - assert!(! tpk.is_null()); - let tpk = unsafe { Box::from_raw(tpk) }; + let tpk = ffi_param_move!(tpk); let packets = unsafe { slice::from_raw_parts_mut(packets, packets_len) }; @@ -205,10 +200,7 @@ pub extern "system" fn sq_tpk_fingerprint(tpk: Option<&TPK>) #[no_mangle] pub extern "system" fn sq_tpk_into_tsk(tpk: *mut TPK) -> *mut TSK { - assert!(!tpk.is_null()); - let tpk = unsafe { - Box::from_raw(tpk) - }; + let tpk = ffi_param_move!(tpk); box_raw!(tpk.into_tsk()) } @@ -352,10 +344,7 @@ pub extern "system" fn sq_tpk_revoke_in_place(ctx: Option<&mut Context>, -> *mut TPK { let ctx = ffi_param_ref!(ctx); - assert!(!tpk.is_null()); - let tpk = unsafe { - Box::from_raw(tpk) - }; + let tpk = ffi_param_move!(tpk); let code = int_to_reason_for_revocation(code); let reason = if let Some(reason) = reason { unsafe { @@ -413,10 +402,7 @@ pub extern "system" fn sq_tpk_set_expiry(ctx: Option<&mut Context>, tpk: *mut TPK, expiry: u32) -> *mut TPK { let ctx = ffi_param_ref!(ctx); - assert!(!tpk.is_null()); - let tpk = unsafe { - Box::from_raw(tpk) - }; + let tpk = ffi_param_move!(tpk); fry_box!(ctx, tpk.set_expiry_in_seconds(expiry)) } @@ -626,8 +612,7 @@ pub extern "system" fn sq_tpk_builder_set_cipher_suite { use self::CipherSuite::*; let tpkb = ffi_param_ref!(tpkb); - assert!(! tpkb.is_null()); - let tpkb_ = unsafe { Box::from_raw(*tpkb) }; + let tpkb_ = ffi_param_move!(*tpkb); let cs = match cs { 0 => Cv25519, 1 => RSA3k, @@ -644,8 +629,7 @@ pub extern "system" fn sq_tpk_builder_add_userid (tpkb: Option<&mut *mut TPKBuilder>, uid: *const c_char) { let tpkb = ffi_param_ref!(tpkb); - assert!(!tpkb.is_null()); - let tpkb_ = unsafe { Box::from_raw(*tpkb) }; + let tpkb_ = ffi_param_move!(*tpkb); let uid = unsafe { CStr::from_ptr(uid).to_string_lossy().to_string() }; let tpkb_ = tpkb_.add_userid(uid.as_ref()); *tpkb = box_raw!(tpkb_); @@ -657,8 +641,7 @@ pub extern "system" fn sq_tpk_builder_add_signing_subkey (tpkb: Option<&mut *mut TPKBuilder>) { let tpkb = ffi_param_ref!(tpkb); - assert!(!tpkb.is_null()); - let tpkb_ = unsafe { Box::from_raw(*tpkb) }; + let tpkb_ = ffi_param_move!(*tpkb); let tpkb_ = tpkb_.add_signing_subkey(); *tpkb = box_raw!(tpkb_); } @@ -669,8 +652,7 @@ pub extern "system" fn sq_tpk_builder_add_encryption_subkey (tpkb: Option<&mut *mut TPKBuilder>) { let tpkb = ffi_param_ref!(tpkb); - assert!(!tpkb.is_null()); - let tpkb_ = unsafe { Box::from_raw(*tpkb) }; + let tpkb_ = ffi_param_move!(*tpkb); let tpkb_ = tpkb_.add_encryption_subkey(); *tpkb = box_raw!(tpkb_); } @@ -681,8 +663,7 @@ pub extern "system" fn sq_tpk_builder_add_certification_subkey (tpkb: Option<&mut *mut TPKBuilder>) { let tpkb = ffi_param_ref!(tpkb); - assert!(!tpkb.is_null()); - let tpkb_ = unsafe { Box::from_raw(*tpkb) }; + let tpkb_ = ffi_param_move!(*tpkb); let tpkb_ = tpkb_.add_certification_subkey(); *tpkb = box_raw!(tpkb_); } @@ -698,10 +679,9 @@ pub extern "system" fn sq_tpk_builder_generate -> Status { let ctx = ffi_param_ref!(ctx); - assert!(!tpkb.is_null()); let tpk_out = ffi_param_ref!(tpk_out); let revocation_out = ffi_param_ref!(revocation_out); - let tpkb = unsafe { Box::from_raw(tpkb) }; + let tpkb = ffi_param_move!(tpkb); match tpkb.generate() { Ok((tpk, revocation)) => { *tpk_out = box_raw!(tpk); diff --git a/ffi/src/store.rs b/ffi/src/store.rs index c7a8944d..31e05376 100644 --- a/ffi/src/store.rs +++ b/ffi/src/store.rs @@ -302,10 +302,7 @@ pub extern "system" fn sq_store_delete(ctx: Option<&mut Context>, store: *mut Store) -> Status { let ctx = ffi_param_ref!(ctx); - assert!(! store.is_null()); - let store = unsafe { - Box::from_raw(store) - }; + let store = ffi_param_move!(store); fry_status!(ctx, store.delete()) } @@ -383,9 +380,7 @@ pub extern "system" fn sq_key_free(key: *mut Key) { #[no_mangle] pub extern "system" fn sq_log_free(log: *mut Log) { if log.is_null() { return }; - let log = unsafe { - Box::from_raw(log) - }; + let log = ffi_param_move!(log); sq_store_free(log.store); sq_binding_free(log.binding); sq_key_free(log.key); @@ -490,10 +485,7 @@ pub extern "system" fn sq_binding_delete(ctx: Option<&mut Context>, binding: *mut Binding) -> Status { let ctx = ffi_param_ref!(ctx); - assert!(! binding.is_null()); - let binding = unsafe { - Box::from_raw(binding) - }; + let binding = ffi_param_move!(binding); fry_status!(ctx, binding.delete()) } -- cgit v1.2.3