From 33748c2046c3ede7e5b79ae9a77944775bd0b9ff Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Tue, 25 Oct 2016 22:34:52 +0200 Subject: [PATCH] Sweep some more panics (#2848) * purge unwraps from ethcrypto, ethstore * sweep panics from util --- Cargo.lock | 1 + ethcrypto/src/lib.rs | 8 ++++++-- ethstore/Cargo.toml | 1 + ethstore/src/dir/disk.rs | 12 +++++++----- ethstore/src/ethstore.rs | 14 +++++++------- ethstore/src/lib.rs | 7 +++++-- ethstore/src/presale.rs | 3 ++- util/src/common.rs | 4 ++-- util/src/journaldb/earlymergedb.rs | 5 ++++- util/src/kvdb.rs | 11 +++++++---- util/src/memorydb.rs | 2 +- util/src/trie/triedb.rs | 5 +++-- util/src/trie/triedbmut.rs | 3 ++- 13 files changed, 48 insertions(+), 28 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 78eabd75e..d6ccbf86f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -613,6 +613,7 @@ dependencies = [ "itertools 0.4.13 (registry+https://github.com/rust-lang/crates.io-index)", "lazy_static 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)", "libc 0.2.15 (registry+https://github.com/rust-lang/crates.io-index)", + "parking_lot 0.3.5 (registry+https://github.com/rust-lang/crates.io-index)", "rand 0.3.14 (registry+https://github.com/rust-lang/crates.io-index)", "rust-crypto 0.2.36 (registry+https://github.com/rust-lang/crates.io-index)", "rustc-serialize 0.3.19 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/ethcrypto/src/lib.rs b/ethcrypto/src/lib.rs index 7a1aba48c..103e750e6 100644 --- a/ethcrypto/src/lib.rs +++ b/ethcrypto/src/lib.rs @@ -166,7 +166,9 @@ pub mod ecies { /// Encrypt a message with a public key pub fn encrypt(public: &Public, shared_mac: &[u8], plain: &[u8]) -> Result, Error> { - let r = Random.generate().unwrap(); + let r = Random.generate() + .expect("context known to have key-generation capabilities; qed"); + let z = try!(ecdh::agree(r.secret(), public)); let mut key = [0u8; 32]; let mut mkey = [0u8; 32]; @@ -201,7 +203,9 @@ pub mod ecies { /// Encrypt a message with a public key pub fn encrypt_single_message(public: &Public, plain: &[u8]) -> Result, Error> { - let r = Random.generate().unwrap(); + let r = Random.generate() + .expect("context known to have key-generation capabilities"); + let z = try!(ecdh::agree(r.secret(), public)); let mut key = [0u8; 32]; let mut mkey = [0u8; 32]; diff --git a/ethstore/Cargo.toml b/ethstore/Cargo.toml index 7fa2a6890..03347cbd7 100644 --- a/ethstore/Cargo.toml +++ b/ethstore/Cargo.toml @@ -18,6 +18,7 @@ docopt = { version = "0.6", optional = true } time = "0.1.34" lazy_static = "0.2" itertools = "0.4" +parking_lot = "0.3" ethcrypto = { path = "../ethcrypto" } [build-dependencies] diff --git a/ethstore/src/dir/disk.rs b/ethstore/src/dir/disk.rs index fe1c46e63..6616ec15d 100644 --- a/ethstore/src/dir/disk.rs +++ b/ethstore/src/dir/disk.rs @@ -28,7 +28,9 @@ const IGNORED_FILES: &'static [&'static str] = &["thumbs.db", "address_book.json fn restrict_permissions_to_owner(file_path: &Path) -> Result<(), i32> { use std::ffi; use libc; - let cstr = ffi::CString::new(file_path.to_str().unwrap()).unwrap(); + + let cstr = try!(ffi::CString::new(&*file_path.to_string_lossy()) + .map_err(|_| -1)); match unsafe { libc::chmod(cstr.as_ptr(), libc::S_IWUSR | libc::S_IRUSR) } { 0 => Ok(()), x => Err(x), @@ -63,15 +65,15 @@ impl DiskDirectory { let paths = try!(fs::read_dir(&self.path)) .flat_map(Result::ok) .filter(|entry| { - let metadata = entry.metadata(); + let metadata = entry.metadata().ok(); let file_name = entry.file_name(); - let name = file_name.to_str().unwrap(); + let name = file_name.to_string_lossy(); // filter directories - metadata.is_ok() && !metadata.unwrap().is_dir() && + metadata.map_or(false, |m| !m.is_dir()) && // hidden files !name.starts_with(".") && // other ignored files - !IGNORED_FILES.contains(&name) + !IGNORED_FILES.contains(&&*name) }) .map(|entry| entry.path()) .collect::>(); diff --git a/ethstore/src/ethstore.rs b/ethstore/src/ethstore.rs index faaea8f9a..4360a39f0 100644 --- a/ethstore/src/ethstore.rs +++ b/ethstore/src/ethstore.rs @@ -15,7 +15,6 @@ // along with Parity. If not, see . use std::collections::BTreeMap; -use std::sync::RwLock; use std::mem; use ethkey::KeyPair; use crypto::KEY_ITERATIONS; @@ -26,6 +25,7 @@ use account::SafeAccount; use {Error, SecretStore}; use json; use json::UUID; +use parking_lot::RwLock; use presale::PresaleWallet; use import; @@ -56,13 +56,13 @@ impl EthStore { let account = try!(self.dir.insert(account.clone())); // update cache - let mut cache = self.cache.write().unwrap(); + let mut cache = self.cache.write(); cache.insert(account.address.clone(), account); Ok(()) } fn reload_accounts(&self) -> Result<(), Error> { - let mut cache = self.cache.write().unwrap(); + let mut cache = self.cache.write(); let accounts = try!(self.dir.load()); let new_accounts: BTreeMap<_, _> = accounts.into_iter().map(|account| (account.address.clone(), account)).collect(); mem::replace(&mut *cache, new_accounts); @@ -71,13 +71,13 @@ impl EthStore { fn get(&self, address: &Address) -> Result { { - let cache = self.cache.read().unwrap(); + let cache = self.cache.read(); if let Some(account) = cache.get(address) { return Ok(account.clone()) } } try!(self.reload_accounts()); - let cache = self.cache.read().unwrap(); + let cache = self.cache.read(); cache.get(address).cloned().ok_or(Error::InvalidAccount) } } @@ -111,7 +111,7 @@ impl SecretStore for EthStore { fn accounts(&self) -> Result, Error> { try!(self.reload_accounts()); - Ok(self.cache.read().unwrap().keys().cloned().collect()) + Ok(self.cache.read().keys().cloned().collect()) } fn change_password(&self, address: &Address, old_password: &str, new_password: &str) -> Result<(), Error> { @@ -131,7 +131,7 @@ impl SecretStore for EthStore { if can_remove { try!(self.dir.remove(address)); - let mut cache = self.cache.write().unwrap(); + let mut cache = self.cache.write(); cache.remove(address); Ok(()) } else { diff --git a/ethstore/src/lib.rs b/ethstore/src/lib.rs index 302e165cf..f8619ff19 100644 --- a/ethstore/src/lib.rs +++ b/ethstore/src/lib.rs @@ -26,12 +26,15 @@ extern crate serde_json; extern crate rustc_serialize; extern crate crypto as rcrypto; extern crate tiny_keccak; -#[macro_use] -extern crate lazy_static; +extern crate parking_lot; + // reexport it nicely extern crate ethkey as _ethkey; extern crate ethcrypto as crypto; +#[macro_use] +extern crate lazy_static; + pub mod dir; pub mod ethkey; diff --git a/ethstore/src/presale.rs b/ethstore/src/presale.rs index 2904db6ef..ff3bde6f4 100644 --- a/ethstore/src/presale.rs +++ b/ethstore/src/presale.rs @@ -33,7 +33,8 @@ impl From for PresaleWallet { impl PresaleWallet { pub fn open

(path: P) -> Result where P: AsRef { let file = try!(fs::File::open(path)); - let presale = json::PresaleWallet::load(file).unwrap(); + let presale = try!(json::PresaleWallet::load(file) + .map_err(|e| Error::InvalidKeyFile(format!("{}", e)))); Ok(PresaleWallet::from(presale)) } diff --git a/util/src/common.rs b/util/src/common.rs index 216f89a79..ea2a0f5ea 100644 --- a/util/src/common.rs +++ b/util/src/common.rs @@ -95,8 +95,8 @@ macro_rules! flushln { #[doc(hidden)] pub fn flush(s: String) { - ::std::io::stdout().write(s.as_bytes()).unwrap(); - ::std::io::stdout().flush().unwrap(); + let _ = ::std::io::stdout().write(s.as_bytes()); + let _ = ::std::io::stdout().flush(); } #[test] diff --git a/util/src/journaldb/earlymergedb.rs b/util/src/journaldb/earlymergedb.rs index ef9868d41..0f7c097b8 100644 --- a/util/src/journaldb/earlymergedb.rs +++ b/util/src/journaldb/earlymergedb.rs @@ -383,7 +383,10 @@ impl JournalDB for EarlyMergeDB { let trace = false; // record new commit's details. - let mut refs = self.refs.as_ref().unwrap().write(); + let mut refs = match self.refs.as_ref() { + Some(refs) => refs.write(), + None => return Ok(0), + }; { let mut index = 0usize; diff --git a/util/src/kvdb.rs b/util/src/kvdb.rs index 63d46d573..b37af4dc9 100644 --- a/util/src/kvdb.rs +++ b/util/src/kvdb.rs @@ -345,7 +345,8 @@ impl Database { let cfnames: Vec<&str> = cfnames.iter().map(|n| n as &str).collect(); match DB::open_cf(&opts, path, &cfnames, &cf_options) { Ok(db) => { - cfs = cfnames.iter().map(|n| db.cf_handle(n).unwrap()).collect(); + cfs = cfnames.iter().map(|n| db.cf_handle(n) + .expect("rocksdb opens a cf_handle for each cfname; qed")).collect(); assert!(cfs.len() == columns as usize); Ok(db) } @@ -353,7 +354,7 @@ impl Database { // retry and create CFs match DB::open_cf(&opts, path, &[], &[]) { Ok(mut db) => { - cfs = cfnames.iter().enumerate().map(|(i, n)| db.create_cf(n, &cf_options[i]).unwrap()).collect(); + cfs = try!(cfnames.iter().enumerate().map(|(i, n)| db.create_cf(n, &cf_options[i])).collect()); Ok(db) }, err @ Err(_) => err, @@ -537,7 +538,8 @@ impl Database { match *self.db.read() { Some(DBAndColumns { ref db, ref cfs }) => { let mut iter = col.map_or_else(|| db.iterator_opt(IteratorMode::From(prefix, Direction::Forward), &self.read_opts), - |c| db.iterator_cf_opt(cfs[c as usize], IteratorMode::From(prefix, Direction::Forward), &self.read_opts).unwrap()); + |c| db.iterator_cf_opt(cfs[c as usize], IteratorMode::From(prefix, Direction::Forward), &self.read_opts) + .expect("iterator params are valid; qed")); match iter.next() { // TODO: use prefix_same_as_start read option (not availabele in C API currently) Some((k, v)) => if k[0 .. prefix.len()] == prefix[..] { Some(v) } else { None }, @@ -554,7 +556,8 @@ impl Database { match *self.db.read() { Some(DBAndColumns { ref db, ref cfs }) => { col.map_or_else(|| DatabaseIterator { iter: db.iterator_opt(IteratorMode::Start, &self.read_opts) }, - |c| DatabaseIterator { iter: db.iterator_cf_opt(cfs[c as usize], IteratorMode::Start, &self.read_opts).unwrap() }) + |c| DatabaseIterator { iter: db.iterator_cf_opt(cfs[c as usize], IteratorMode::Start, &self.read_opts) + .expect("iterator params are valid; qed") }) }, None => panic!("Not supported yet") //TODO: return an empty iterator or change return type } diff --git a/util/src/memorydb.rs b/util/src/memorydb.rs index 36e3fad1b..d0ecb73d9 100644 --- a/util/src/memorydb.rs +++ b/util/src/memorydb.rs @@ -148,7 +148,7 @@ impl MemoryDB { (*p).insert(key.clone(), (value, 0)); } } - self.raw(key).unwrap() + self.raw(key).expect("entry just inserted into data; qed") } /// Returns the size of allocated heap memory diff --git a/util/src/trie/triedb.rs b/util/src/trie/triedb.rs index 5522af448..72604892d 100644 --- a/util/src/trie/triedb.rs +++ b/util/src/trie/triedb.rs @@ -296,7 +296,7 @@ impl<'a> TrieDBIterator<'a> { status: Status::Entering, node: try!(self.db.get_node(d, &mut NoOp, 0)), }); - match self.trail.last().unwrap().node { + match self.trail.last().expect("just pushed item; qed").node { Node::Leaf(n, _) | Node::Extension(n, _) => { self.key_nibbles.extend(n.iter()); }, _ => {} } @@ -346,7 +346,8 @@ impl<'a> Iterator for TrieDBIterator<'a> { (Status::AtChild(i), Node::Branch(children, _)) if children[i].len() > 0 => { match i { 0 => self.key_nibbles.push(0), - i => *self.key_nibbles.last_mut().unwrap() = i as u8, + i => *self.key_nibbles.last_mut() + .expect("pushed as 0; moves sequentially; removed afterwards; qed") = i as u8, } if let Err(e) = self.descend(children[i]) { return Some(Err(e)); diff --git a/util/src/trie/triedbmut.rs b/util/src/trie/triedbmut.rs index 5e2ef6b79..e99179e7a 100644 --- a/util/src/trie/triedbmut.rs +++ b/util/src/trie/triedbmut.rs @@ -744,7 +744,8 @@ impl<'a> TrieDBMut<'a> { (UsedIndex::One(a), None) => { // only one onward node. make an extension. let new_partial = NibbleSlice::new_offset(&[a], 1).encoded(false); - let new_node = Node::Extension(new_partial, children[a as usize].take().unwrap()); + let child = children[a as usize].take().expect("used_index only set if occupied; qed"); + let new_node = Node::Extension(new_partial, child); self.fix(new_node) } (UsedIndex::None, Some(value)) => {