Sweep some more panics (#2848)

* purge unwraps from ethcrypto, ethstore

* sweep panics from util

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)",

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<Vec<u8>, 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<Vec<u8>, 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];

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]

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::<Vec<PathBuf>>();

ethstore/src/ethstore.rs

@@ -15,7 +15,6 @@
 // along with Parity.  If not, see <http://www.gnu.org/licenses/>.
 
 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<SafeAccount, Error> {
 		{
-			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<Vec<Address>, 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 {

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;
 

ethstore/src/presale.rs

@@ -33,7 +33,8 @@ impl From<json::PresaleWallet> for PresaleWallet {
 impl PresaleWallet {
 	pub fn open<P>(path: P) -> Result<Self, Error> where P: AsRef<Path> {
 		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))
 	}
 

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]

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;

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
 		}

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

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));

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)) => {