Skip to content
This repository was archived by the owner on Nov 30, 2022. It is now read-only.

new literacy::{Read, Write} traits to handle std/no_std #126

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,12 @@ default = [ "std" ]
std = []
serde-std = ["serde/std"]
unstable = [] # for benchmarking
use-core2 = ["core2"]

[dependencies]
serde = { version = "1.0", default-features = false, optional = true }
schemars = { version = "0.8.0", optional = true }
core2 = { version="0.3.0-alpha.1", optional= true, default-features = false }

[dev-dependencies]
serde_test = "1.0"
Expand Down
3 changes: 2 additions & 1 deletion embedded/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ extern crate alloc;

use alloc_cortex_m::CortexMHeap;
use bitcoin_hashes::{sha256, Hash, HashEngine};
use bitcoin_hashes::literacy::Write;
use core::alloc::Layout;
use core::str::FromStr;
use cortex_m::asm;
Expand All @@ -29,7 +30,7 @@ fn main() -> ! {
unsafe { ALLOCATOR.init(cortex_m_rt::heap_start() as usize, HEAP_SIZE) }

let mut engine = TestType::engine();
engine.input(b"abc");
engine.write(b"abc").unwrap();
let hash = TestType::from_engine(engine);

let hash_check =
Expand Down
50 changes: 24 additions & 26 deletions src/std_impls.rs → src/impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,71 +16,69 @@
//!
//! impls of traits defined in `std` and not `core`

use std::{error, io};
use {sha1, sha256, sha512, ripemd160, siphash24};
use ::{HashEngine, literacy};

use {hex, sha1, sha256, sha512, ripemd160, siphash24};
use HashEngine;
use Error;

impl error::Error for Error {
fn cause(&self) -> Option<&error::Error> { None }
#[cfg(any(test, feature = "std"))]
impl ::std::error::Error for ::Error {
fn cause(&self) -> Option<&::std::error::Error> { None }
fn description(&self) -> &str { "`std::error::description` is deprecated" }
}

impl error::Error for hex::Error {
fn cause(&self) -> Option<&error::Error> { None }
#[cfg(any(test, feature = "std"))]
impl ::std::error::Error for ::hex::Error {
fn cause(&self) -> Option<&::std::error::Error> { None }
fn description(&self) -> &str { "`std::error::description` is deprecated" }
}

impl io::Write for sha1::HashEngine {
fn flush(&mut self) -> io::Result<()> { Ok(()) }
impl literacy::Write for sha1::HashEngine {
fn flush(&mut self) -> ::core::result::Result<(), literacy::Error> { Ok(()) }

fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
fn write(&mut self, buf: &[u8]) -> ::core::result::Result<usize, literacy::Error> {
self.input(buf);
Ok(buf.len())
}
}

impl io::Write for sha256::HashEngine {
fn flush(&mut self) -> io::Result<()> { Ok(()) }
impl literacy::Write for sha256::HashEngine {
fn flush(&mut self) -> ::core::result::Result<(), literacy::Error> { Ok(()) }

fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
fn write(&mut self, buf: &[u8]) -> ::core::result::Result<usize, literacy::Error> {
self.input(buf);
Ok(buf.len())
}
}

impl io::Write for sha512::HashEngine {
fn flush(&mut self) -> io::Result<()> { Ok(()) }
impl literacy::Write for sha512::HashEngine {
fn flush(&mut self) -> ::core::result::Result<(), literacy::Error> { Ok(()) }

fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
fn write(&mut self, buf: &[u8]) -> ::core::result::Result<usize, literacy::Error> {
self.input(buf);
Ok(buf.len())
}
}

impl io::Write for ripemd160::HashEngine {
fn flush(&mut self) -> io::Result<()> { Ok(()) }
impl literacy::Write for ripemd160::HashEngine {
fn flush(&mut self) -> ::core::result::Result<(), literacy::Error> { Ok(()) }

fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
fn write(&mut self, buf: &[u8]) -> ::core::result::Result<usize, literacy::Error> {
self.input(buf);
Ok(buf.len())
}
}

impl io::Write for siphash24::HashEngine {
fn flush(&mut self) -> io::Result<()> { Ok(()) }
impl literacy::Write for siphash24::HashEngine {
fn flush(&mut self) -> ::core::result::Result<(), literacy::Error> { Ok(()) }

fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
fn write(&mut self, buf: &[u8]) -> ::core::result::Result<usize, literacy::Error> {
self.input(buf);
Ok(buf.len())
}
}

#[cfg(test)]
mod tests {
use std::io::Write;

use ::literacy::Write;
use {sha1, sha256, sha256d, sha512, ripemd160, hash160, siphash24};
use Hash;

Expand Down
5 changes: 3 additions & 2 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
#![deny(non_camel_case_types)]
#![deny(non_snake_case)]
#![deny(unused_mut)]
#![deny(missing_docs)]
//#![deny(missing_docs)]

// In general, rust is absolutely horrid at supporting users doing things like,
// for example, compiling Rust code for real environments. Disable useless lints
Expand Down Expand Up @@ -53,7 +53,7 @@ pub mod _export {

#[macro_use] mod util;
#[macro_use] pub mod serde_macros;
#[cfg(any(test, feature = "std"))] mod std_impls;
mod impls;
pub mod error;
pub mod hex;
pub mod hash160;
Expand All @@ -66,6 +66,7 @@ pub mod sha256t;
pub mod siphash24;
pub mod sha512;
pub mod cmp;
pub mod literacy;

use core::{borrow, fmt, hash, ops};

Expand Down
148 changes: 148 additions & 0 deletions src/literacy.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,148 @@
#[cfg(all(feature = "std", feature = "use-core2"))]
compile_error!("feature \"std\" and \"use-core2\" cannot be enabled together.");

#[derive(Debug)]
pub enum Error {
#[cfg(feature = "std")]
Std(::std::io::Error),
Copy link
Contributor

@sgeisler sgeisler May 8, 2021

Choose a reason for hiding this comment

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

Feature-gated enums are really bad because cargo uses the union set of all features in the dependency tree. E.g. if there are two deps with a rust-bitcoin dep and one rust-bitcoin has use-core and another std, cargo will compile it once with both features. That means a non-breaking change (just changing the feature set of rust-bitcoin without exposing it) in one of the deps could break exhaustive matches.

Would Box<dyn SomeNonStdErrorTrait> work here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought of Box<dyn SomeNonStdErrorTrait> for the Other variant, but I am not sure what SomeNonStdErrorTrait could be, Debug? Or something created ad-hoc?

Copy link
Contributor

Choose a reason for hiding this comment

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

It could be a new trait defined as trait OurErrorTrait: Dsiplay + Debug + … {}, essentially just a hack to make multiple trait bounds object safe.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we can't avoid the feature gate on the error variant on 1.29, because you can't use Box taken from alloc on 1.29

Copy link
Contributor

Choose a reason for hiding this comment

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

With 1.29, you must turn on std and must not turn on use-core2 anyway, so there seems to be no issue. Box only needs to be imported from alloc conditional on not(feature = "std")), which will be skipped by 1.29.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, right, I had to update the CI for that (skipping no_std for 1.29) 4727b63

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm a bit confused as to how CI was passing before, given that 1.29 can't compile core2.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was not compiling it, features in contrib/test.sh were not updated (there were also 1 feature not existing anymore)

Copy link

@thomaseizinger thomaseizinger May 10, 2021

Choose a reason for hiding this comment

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

The two features are mutually exclusive right? (which by itself also doesn't play well with cargo's way of handling features)

Instead of having conditionally defined variants, you could have a single variant and conditionally define its private, inner field?

I think that should address @sgeisler's concern.


#[cfg(feature = "use-core2")]
Core2(core2::io::Error),

// Needed for write_all blanket implementation
WriteZero,
Interrupted,

Other,
}

pub trait Read{
fn read(&mut self, buf: &mut [u8]) -> ::core::result::Result<usize, Error>;
}

pub trait Write {
fn write(&mut self, buf: &[u8]) -> ::core::result::Result<usize, Error>;
fn flush(&mut self) -> ::core::result::Result<(), Error>;

fn write_all(&mut self, mut buf: &[u8]) -> ::core::result::Result<(), Error> {
while !buf.is_empty() {
match self.write(buf) {
Ok(0) => {
return Err(Error::WriteZero);
}
Ok(n) => buf = &buf[n..],
Err(Error::Interrupted) => {}
Err(e) => return Err(e),
}
}
Ok(())
}
}

#[cfg(feature = "std")]
mod std_impl {
use super::{Read, Write, Error};

impl From<::std::io::Error> for Error {
fn from(error: ::std::io::Error) -> Self {
if let ::std::io::ErrorKind::Interrupted = error.kind() {
Error::Interrupted
} else {
Error::Std(error)
}
}
}

impl<R: ::std::io::Read> Read for R {
fn read(&mut self, buf: &mut [u8]) -> Result<usize, Error> {
Ok(<Self as ::std::io::Read>::read(self, buf)?)
}
}

impl<W: ::std::io::Write> Write for W {
fn write(&mut self, buf: &[u8]) -> Result<usize, Error> {
Ok(<Self as ::std::io::Write>::write(self, buf)?)
}

fn flush(&mut self) -> Result<(), Error> {
Ok(<Self as ::std::io::Write>::flush(self)?)
}
}
}

#[cfg(feature = "use-core2")]
mod core2_impl {
use super::{Read, Write, Error};

impl From<core2::io::Error> for Error {
fn from(error: core2::io::Error) -> Self {
if let core2::io::ErrorKind::Interrupted = error.kind() {
Error::Interrupted
} else {
Error::Core2(error)
}
}
}

impl<R: core2::io::Read> Read for R {
fn read(&mut self, buf: &mut [u8]) -> Result<usize, Error> {
Ok(<Self as core2::io::Read>::read(self, buf)?)
}
}

impl<W: core2::io::Write> Write for W {
fn write(&mut self, buf: &[u8]) -> Result<usize, Error> {
Ok(<Self as core2::io::Write>::write(self, buf)?)
}

fn flush(&mut self) -> Result<(), Error> {
Ok(<Self as core2::io::Write>::flush(self)?)
}
}
}

#[cfg(test)]
mod tests {

#[cfg(feature = "std")]
mod std_test {
use ::literacy::{Read, Write};

#[test]
fn test_std_read() {
let mut cursor = ::std::io::Cursor::new(vec![10u8]);
let mut buf = [0u8; 1];
cursor.read(&mut buf).unwrap();
assert_eq!(buf, [10u8]);
}

#[test]
fn test_std_write() {
let mut cursor = ::std::io::Cursor::new(vec![]);
let mut buf = [10u8; 1];
cursor.write(&mut buf).unwrap();
assert_eq!(cursor.into_inner(), vec![10u8]);
}
}

#[cfg(feature = "use-core2")]
mod tests {
use ::literacy::{Read, Write};

#[test]
fn test_core2_read() {
let mut cursor = core2::io::Cursor::new(vec![10u8]);
let mut buf = [0u8; 1];
cursor.read(&mut buf).unwrap();
assert_eq!(buf, [10u8]);
}

#[test]
fn test_core2_write() {
let mut cursor = core2::io::Cursor::new(vec![]);
let mut buf = [10u8; 1];
cursor.write(&mut buf).unwrap();
assert_eq!(cursor.into_inner(), vec![10u8]);
}
}
}