Skip to content

Commit 04ce508

Browse files
apoelstraDavidson-Souza
authored andcommitted
lib: fix bad unit test
1 parent e4cca90 commit 04ce508

File tree

1 file changed

+37
-8
lines changed

1 file changed

+37
-8
lines changed

src/lib.rs

+37-8
Original file line numberDiff line numberDiff line change
@@ -591,15 +591,23 @@ mod tests {
591591

592592
#[test]
593593
#[cfg(feature = "rand-std")]
594+
// In rustc 1.72 this Clippy lint was pulled out of clippy and into rustc, and
595+
// was made deny-by-default, breaking compilation of this test. Aside from this
596+
// breaking change, which there is no point in bugging, the rename was done so
597+
// clumsily that you need four separate "allow"s to disable this wrong lint.
598+
#[allow(unknown_lints)]
599+
#[allow(renamed_and_removed_lints)]
600+
#[allow(undropped_manually_drops)]
601+
#[allow(clippy::unknown_manually_drops)]
594602
fn test_raw_ctx() {
595-
use std::mem::ManuallyDrop;
603+
use std::mem::{forget, ManuallyDrop};
596604

597605
let ctx_full = Secp256k1::new();
598606
let ctx_sign = Secp256k1::signing_only();
599607
let ctx_vrfy = Secp256k1::verification_only();
600608

601-
let mut full = unsafe { Secp256k1::from_raw_all(ctx_full.ctx) };
602-
let mut sign = unsafe { Secp256k1::from_raw_signing_only(ctx_sign.ctx) };
609+
let full = unsafe { Secp256k1::from_raw_all(ctx_full.ctx) };
610+
let sign = unsafe { Secp256k1::from_raw_signing_only(ctx_sign.ctx) };
603611
let mut vrfy = unsafe { Secp256k1::from_raw_verification_only(ctx_vrfy.ctx) };
604612

605613
let (sk, pk) = full.generate_keypair(&mut rand::thread_rng());
@@ -612,14 +620,35 @@ mod tests {
612620
assert!(vrfy.verify_ecdsa(&msg, &sig, &pk).is_ok());
613621
assert!(full.verify_ecdsa(&msg, &sig, &pk).is_ok());
614622

623+
// The following drop will have no effect; in fact, they will trigger a compiler
624+
// error because manually dropping a `ManuallyDrop` is almost certainly incorrect.
625+
// If you want to drop the inner object you should called `ManuallyDrop::drop`.
626+
drop(full);
627+
// This will actually drop the context, though it will leave `full` accessible and
628+
// in an invalid state. However, this is almost certainly what you want to do.
629+
drop(ctx_full);
630+
unsafe {
631+
// Need to compute the allocation size, and need to do so *before* dropping
632+
// anything.
633+
let sz = ffi::secp256k1_context_preallocated_clone_size(ctx_sign.ctx.as_ptr());
634+
// We can alternately drop the `ManuallyDrop` by unwrapping it and then letting
635+
// it be dropped. This is actually a safe function, but it will destruct the
636+
// underlying context without deallocating it...
637+
ManuallyDrop::into_inner(sign);
638+
// ...leaving us holding the bag to deallocate the context's memory without
639+
// double-calling `secp256k1_context_destroy`, which cannot be done safely.
640+
SignOnly::deallocate(ctx_sign.ctx.as_ptr() as *mut u8, sz);
641+
forget(ctx_sign);
642+
}
643+
615644
unsafe {
616-
ManuallyDrop::drop(&mut full);
617-
ManuallyDrop::drop(&mut sign);
645+
// Finally, we can call `ManuallyDrop::drop`, which has the same effect, but
646+
let sz = ffi::secp256k1_context_preallocated_clone_size(ctx_vrfy.ctx.as_ptr());
647+
// leaves the `ManuallyDrop` itself accessible. This is marked unsafe.
618648
ManuallyDrop::drop(&mut vrfy);
649+
VerifyOnly::deallocate(ctx_vrfy.ctx.as_ptr() as *mut u8, sz);
650+
forget(ctx_vrfy);
619651
}
620-
drop(ctx_full);
621-
drop(ctx_sign);
622-
drop(ctx_vrfy);
623652
}
624653

625654
#[cfg(not(target_arch = "wasm32"))]

0 commit comments

Comments
 (0)