From 87cca9a78e9ca32303dc99391f1317c1410cc118 Mon Sep 17 00:00:00 2001 From: Michal Nazarewicz Date: Fri, 17 Nov 2023 21:24:11 +0100 Subject: [PATCH] Take advantage of a new std::io::Error::other (#10198) Now that std::io::Error::other has stabilised, use it to simplify bunch of code. --- core/store/src/db/colddb.rs | 2 +- core/store/src/db/rocksdb.rs | 24 +++++---------- core/store/src/db/rocksdb/instance_tracker.rs | 8 ++--- core/store/src/db/rocksdb/snapshot.rs | 4 +-- core/store/src/flat/store_helper.rs | 12 +++----- core/store/src/metadata.rs | 25 ++++++---------- core/store/src/opener.rs | 2 +- runtime/near-test-contracts/build.rs | 30 +++++++++---------- 8 files changed, 42 insertions(+), 65 deletions(-) diff --git a/core/store/src/db/colddb.rs b/core/store/src/db/colddb.rs index 90aa9e7ac5d..57c2a2903d9 100644 --- a/core/store/src/db/colddb.rs +++ b/core/store/src/db/colddb.rs @@ -29,7 +29,7 @@ impl ColdDB { // Checks if the column is is the cold db and returns an error if not. fn check_is_in_colddb(col: DBCol) -> std::io::Result<()> { if !col.is_in_colddb() { - return Err(std::io::Error::new(std::io::ErrorKind::Other, Self::err_msg(col))); + return Err(std::io::Error::other(Self::err_msg(col))); } Ok(()) } diff --git a/core/store/src/db/rocksdb.rs b/core/store/src/db/rocksdb.rs index 35e73a6c156..0a6e0a12771 100644 --- a/core/store/src/db/rocksdb.rs +++ b/core/store/src/db/rocksdb.rs @@ -114,7 +114,7 @@ impl RocksDB { columns: &[DBCol], ) -> io::Result { let counter = instance_tracker::InstanceTracker::try_new(store_config.max_open_files) - .map_err(other_error)?; + .map_err(io::Error::other)?; let (db, db_opt) = Self::open_db(path, store_config, mode, temp, columns)?; let cf_handles = Self::get_cf_handles(&db, columns); Ok(Self { db, db_opt, cf_handles, _instance_tracker: counter }) @@ -144,7 +144,7 @@ impl RocksDB { } else { DB::open_cf_descriptors(&options, path, cf_descriptors) } - .map_err(into_other)?; + .map_err(io::Error::other)?; if cfg!(feature = "single_thread_rocksdb") { // These have to be set after open db let mut env = Env::new().unwrap(); @@ -200,7 +200,7 @@ impl RocksDB { } else if cfg!(debug_assertions) { panic!("The database instance isn’t setup to access {col}"); } else { - Err(other_error(format!("{col}: no such column"))) + Err(io::Error::other(format!("{col}: no such column"))) } } @@ -269,7 +269,7 @@ impl<'a> Iterator for RocksDBIterator<'a> { type Item = io::Result<(Box<[u8]>, Box<[u8]>)>; fn next(&mut self) -> Option { - Some(self.0.next()?.map_err(into_other)) + Some(self.0.next()?.map_err(io::Error::other)) } } @@ -314,7 +314,7 @@ impl Database for RocksDB { let result = self .db .get_pinned_cf_opt(self.cf_handle(col)?, key, &read_options) - .map_err(into_other)? + .map_err(io::Error::other)? .map(DBSlice::from_rocksdb_slice); timer.observe_duration(); Ok(result) @@ -366,7 +366,7 @@ impl Database for RocksDB { } DBOp::DeleteAll { col } => { let cf_handle = self.cf_handle(col)?; - let range = self.get_cf_key_range(cf_handle).map_err(into_other)?; + let range = self.get_cf_key_range(cf_handle).map_err(io::Error::other)?; if let Some(range) = range { batch.delete_range_cf(cf_handle, range.start(), range.end()); // delete_range_cf deletes ["begin_key", "end_key"), so need one more delete @@ -378,7 +378,7 @@ impl Database for RocksDB { } } } - self.db.write(batch).map_err(into_other) + self.db.write(batch).map_err(io::Error::other) } fn compact(&self) -> io::Result<()> { @@ -392,7 +392,7 @@ impl Database for RocksDB { // Need to iterator over all CFs because the normal `flush()` only // flushes the default column family. for col in DBCol::iter() { - self.db.flush_cf(self.cf_handle(col)?).map_err(into_other)?; + self.db.flush_cf(self.cf_handle(col)?).map_err(io::Error::other)?; } Ok(()) } @@ -640,14 +640,6 @@ fn parse_statistics( Ok(()) } -fn other_error(msg: String) -> io::Error { - io::Error::new(io::ErrorKind::Other, msg) -} - -fn into_other(error: rocksdb::Error) -> io::Error { - io::Error::new(io::ErrorKind::Other, error.into_string()) -} - /// Returns name of a RocksDB column family corresponding to given column. /// /// Historically we used `col##` names (with `##` being index of the column). diff --git a/core/store/src/db/rocksdb/instance_tracker.rs b/core/store/src/db/rocksdb/instance_tracker.rs index 784bbfd65c5..31d18880224 100644 --- a/core/store/src/db/rocksdb/instance_tracker.rs +++ b/core/store/src/db/rocksdb/instance_tracker.rs @@ -205,17 +205,13 @@ impl NoFile for RealNoFile { #[test] fn test_ensure_max_open_files_limit() { - fn other_error(msg: &str) -> std::io::Error { - super::other_error(msg.to_string()) - } - /// Mock implementation of NoFile interface. struct MockNoFile<'a>(&'a mut (u64, u64)); impl<'a> NoFile for MockNoFile<'a> { fn get(&self) -> std::io::Result<(u64, u64)> { if self.0 .0 == 666 { - Err(other_error("error")) + Err(std::io::ErrorKind::Other.into()) } else { Ok(*self.0) } @@ -224,7 +220,7 @@ fn test_ensure_max_open_files_limit() { fn set(&mut self, soft: u64, hard: u64) -> std::io::Result<()> { let (old_soft, old_hard) = self.get().unwrap(); if old_hard == 666000 { - Err(other_error("error")) + Err(std::io::ErrorKind::Other.into()) } else { assert!(soft != old_soft, "Pointless call to set"); *self.0 = (soft, hard); diff --git a/core/store/src/db/rocksdb/snapshot.rs b/core/store/src/db/rocksdb/snapshot.rs index d36c27ad95f..76be0806b55 100644 --- a/core/store/src/db/rocksdb/snapshot.rs +++ b/core/store/src/db/rocksdb/snapshot.rs @@ -51,7 +51,7 @@ impl std::convert::From for SnapshotError { impl std::convert::From<::rocksdb::Error> for SnapshotError { fn from(err: ::rocksdb::Error) -> Self { - super::into_other(err).into() + io::Error::other(err).into() } } @@ -94,7 +94,7 @@ impl Snapshot { } let db = super::RocksDB::open(db_path, config, crate::Mode::ReadWriteExisting, temp)?; - let cp = Checkpoint::new(&db.db).map_err(super::into_other)?; + let cp = Checkpoint::new(&db.db).map_err(io::Error::other)?; cp.create_checkpoint(&snapshot_path)?; Ok(Self(Some(snapshot_path))) diff --git a/core/store/src/flat/store_helper.rs b/core/store/src/flat/store_helper.rs index d09b9300469..3c4510634db 100644 --- a/core/store/src/flat/store_helper.rs +++ b/core/store/src/flat/store_helper.rs @@ -139,17 +139,13 @@ pub fn encode_flat_state_db_key(shard_uid: ShardUId, key: &[u8]) -> Vec { pub fn decode_flat_state_db_key(key: &[u8]) -> io::Result<(ShardUId, Vec)> { if key.len() < 8 { - return Err(io::Error::new( - io::ErrorKind::Other, - format!("expected FlatState key length to be at least 8: {key:?}"), - )); + return Err(io::Error::other(format!( + "expected FlatState key length to be at least 8: {key:?}" + ))); } let (shard_uid_bytes, trie_key) = key.split_at(8); let shard_uid = shard_uid_bytes.try_into().map_err(|err| { - io::Error::new( - io::ErrorKind::Other, - format!("failed to decode shard_uid as part of FlatState key: {err}"), - ) + io::Error::other(format!("failed to decode shard_uid as part of FlatState key: {err}")) })?; Ok((shard_uid, trie_key.to_vec())) } diff --git a/core/store/src/metadata.rs b/core/store/src/metadata.rs index eab2c05041c..30a25a20ef3 100644 --- a/core/store/src/metadata.rs +++ b/core/store/src/metadata.rs @@ -103,7 +103,7 @@ fn read( match result { Some(value) => Ok(value), - None => Err(other_error(format!("missing {what}; {msg}"))), + None => Err(std::io::Error::other(format!("missing {what}; {msg}"))), } } @@ -118,19 +118,12 @@ fn maybe_read( key: &[u8], ) -> std::io::Result> { let msg = "it’s not a neard database or database is corrupted"; - if let Some(bytes) = db.get_raw_bytes(crate::DBCol::DbVersion, key)? { - let value = std::str::from_utf8(&bytes) - .map_err(|_err| format!("invalid {what}: {bytes:?}; {msg}")) - .map_err(other_error)?; - let value = T::from_str(value) - .map_err(|_err| format!("invalid {what}: ‘{value}’; {msg}")) - .map_err(other_error)?; - Ok(Some(value)) - } else { - Ok(None) - } -} - -fn other_error(msg: String) -> std::io::Error { - std::io::Error::new(std::io::ErrorKind::Other, msg) + db.get_raw_bytes(crate::DBCol::DbVersion, key)? + .map(|bytes| { + let value = std::str::from_utf8(&bytes) + .map_err(|_err| format!("invalid {what}: {bytes:?}; {msg}"))?; + T::from_str(value).map_err(|_err| format!("invalid {what}: ‘{value}’; {msg}")) + }) + .transpose() + .map_err(std::io::Error::other) } diff --git a/core/store/src/opener.rs b/core/store/src/opener.rs index 6a5926758e5..a4c59e4637f 100644 --- a/core/store/src/opener.rs +++ b/core/store/src/opener.rs @@ -524,7 +524,7 @@ impl<'a> DBOpener<'a> { let metadata = DbMetadata::read(&db)?; if want_version != metadata.version { let msg = format!("unexpected DbVersion {}; expected {want_version}", metadata.version); - Err(std::io::Error::new(std::io::ErrorKind::Other, msg)) + Err(std::io::Error::other(msg)) } else { Ok((db, metadata)) } diff --git a/runtime/near-test-contracts/build.rs b/runtime/near-test-contracts/build.rs index e551f5ff07b..41e062e8bb6 100644 --- a/runtime/near-test-contracts/build.rs +++ b/runtime/near-test-contracts/build.rs @@ -1,14 +1,11 @@ -use std::path::Path; -use std::path::PathBuf; use std::process::Command; -use std::{env, fs, io, process}; type Error = Box; fn main() { if let Err(err) = try_main() { eprintln!("{}", err); - process::exit(1); + std::process::exit(1); } } @@ -39,14 +36,14 @@ fn build_contract(dir: &str, args: &[&str], output: &str) -> Result<(), Error> { let src = target_dir.join(format!("wasm32-unknown-unknown/release/{}.wasm", dir.replace('-', "_"))); - fs::copy(&src, format!("./res/{}.wasm", output)) + std::fs::copy(&src, format!("./res/{}.wasm", output)) .map_err(|err| format!("failed to copy `{}`: {}", src.display(), err))?; println!("cargo:rerun-if-changed=./{}/src/lib.rs", dir); println!("cargo:rerun-if-changed=./{}/Cargo.toml", dir); Ok(()) } -fn cargo_build_cmd(target_dir: &Path) -> Command { +fn cargo_build_cmd(target_dir: &std::path::Path) -> Command { let mut res = Command::new("cargo"); res.env_remove("CARGO_BUILD_RUSTFLAGS"); @@ -61,15 +58,18 @@ fn cargo_build_cmd(target_dir: &Path) -> Command { } fn check_status(mut cmd: Command) -> Result<(), Error> { - let status = cmd.status().map_err(|err| { - io::Error::new(io::ErrorKind::Other, format!("command `{:?}` failed to run: {}", cmd, err)) - })?; - if !status.success() { - return Err(format!("command `{:?}` exited with non-zero status: {:?}", cmd, status).into()); - } - Ok(()) + cmd.status() + .map_err(|err| format!("command `{cmd:?}` failed to run: {err}")) + .and_then(|status| { + if status.success() { + Ok(()) + } else { + Err(format!("command `{cmd:?}` exited with non-zero status: {status:?}")) + } + }) + .map_err(Error::from) } -fn out_dir() -> PathBuf { - env::var("OUT_DIR").unwrap().into() +fn out_dir() -> std::path::PathBuf { + std::env::var("OUT_DIR").unwrap().into() }