From 946cd83106ea05fd55a2560309cae97921a5dc4f Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Tue, 14 Jun 2022 09:33:48 +1000 Subject: [PATCH] Improve Error display The current display code for `Error` is a little unusual. We typically just implement `Display` and if a `str` is needed use `format!`. Improve the `Error` type by doing - Remove the `as_str` function and implement `Display` directly. - Remove the 'secp: ' prefix of all the error messages. - Use a newly defined macro `write_err` that writes the error if `std` feature is not enabled so that no-std builds do not loose error info. Note: The `write_err` macro is currently being introduced in `rust-bitcoin` also. Elect to just duplicate it here and not share it between the crates. --- src/lib.rs | 35 +++++++++++++++-------------------- src/macros.rs | 19 +++++++++++++++++++ 2 files changed, 34 insertions(+), 20 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index f1cc95952..327d4aff7 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -349,28 +349,23 @@ pub enum Error { InvalidParityValue(key::InvalidParityValue), } -impl Error { - fn as_str(&self) -> &str { - match *self { - Error::IncorrectSignature => "secp: signature failed verification", - Error::InvalidMessage => "secp: message was not 32 bytes (do you need to hash?)", - Error::InvalidPublicKey => "secp: malformed public key", - Error::InvalidSignature => "secp: malformed signature", - Error::InvalidSecretKey => "secp: malformed or out-of-range secret key", - Error::InvalidSharedSecret => "secp: malformed or out-of-range shared secret", - Error::InvalidRecoveryId => "secp: bad recovery id", - Error::InvalidTweak => "secp: bad tweak", - Error::NotEnoughMemory => "secp: not enough memory allocated", - Error::InvalidPublicKeySum => "secp: the sum of public keys was invalid or the input vector lengths was less than 1", - Error::InvalidParityValue(_) => "couldn't create parity", - } - } -} - -// Passthrough Debug to Display, since errors should be user-visible. impl fmt::Display for Error { fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> { - f.write_str(self.as_str()) + use Error::*; + + match *self { + IncorrectSignature => f.write_str("signature failed verification"), + InvalidMessage => f.write_str("message was not 32 bytes (do you need to hash?)"), + InvalidPublicKey => f.write_str("malformed public key"), + InvalidSignature => f.write_str("malformed signature"), + InvalidSecretKey => f.write_str("malformed or out-of-range secret key"), + InvalidSharedSecret => f.write_str("malformed or out-of-range shared secret"), + InvalidRecoveryId => f.write_str("bad recovery id"), + InvalidTweak => f.write_str("bad tweak"), + NotEnoughMemory => f.write_str("not enough memory allocated"), + InvalidPublicKeySum => f.write_str("the sum of public keys was invalid or the input vector lengths was less than 1"), + InvalidParityValue(e) => write_err!(f, "couldn't create parity"; e), + } } } diff --git a/src/macros.rs b/src/macros.rs index 6cd1d66a9..152baa957 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -26,3 +26,22 @@ macro_rules! impl_pretty_debug { } } } + +/// Formats error. If `std` feature is OFF appends error source (delimited by `: `). We do this +/// because `e.source()` is only available in std builds, without this macro the error source is +/// lost for no-std builds. +macro_rules! write_err { + ($writer:expr, $string:literal $(, $args:expr),*; $source:expr) => { + { + #[cfg(feature = "std")] + { + let _ = &$source; // Prevents clippy warnings. + write!($writer, $string $(, $args)*) + } + #[cfg(not(feature = "std"))] + { + write!($writer, concat!($string, ": {}") $(, $args)*, $source) + } + } + } +}