From 83750719c1748b44d91ca85e8449d5294daa74a4 Mon Sep 17 00:00:00 2001 From: Michal Nazarewicz Date: Wed, 22 Jun 2022 18:01:45 +0200 Subject: [PATCH] Consistently use ffi_util::to_cpath to convert Path to CString (#640) --- src/backup.rs | 60 +++++------------------------------------------ src/checkpoint.rs | 16 +++---------- src/db_options.rs | 12 +++++----- 3 files changed, 15 insertions(+), 73 deletions(-) diff --git a/src/backup.rs b/src/backup.rs index eb26a0c..cb84a5a 100644 --- a/src/backup.rs +++ b/src/backup.rs @@ -13,9 +13,8 @@ // limitations under the License. // -use crate::{ffi, Error, DB}; +use crate::{ffi, ffi_util::to_cpath, Error, DB}; -use std::ffi::CString; use std::path::Path; /// Represents information of a backup including timestamp of the backup @@ -48,16 +47,7 @@ pub struct RestoreOptions { impl BackupEngine { /// Open a backup engine with the specified options. pub fn open>(opts: &BackupEngineOptions, path: P) -> Result { - let path = path.as_ref(); - let cpath = if let Ok(e) = CString::new(path.to_string_lossy().as_bytes()) { - e - } else { - return Err(Error::new( - "Failed to convert path to CString \ - when opening backup engine" - .to_owned(), - )); - }; + let cpath = to_cpath(path)?; let be: *mut ffi::rocksdb_backup_engine_t; unsafe { @@ -136,27 +126,8 @@ impl BackupEngine { wal_dir: W, opts: &RestoreOptions, ) -> Result<(), Error> { - let db_dir = db_dir.as_ref(); - let c_db_dir = if let Ok(c) = CString::new(db_dir.to_string_lossy().as_bytes()) { - c - } else { - return Err(Error::new( - "Failed to convert db_dir to CString \ - when restoring from latest backup" - .to_owned(), - )); - }; - - let wal_dir = wal_dir.as_ref(); - let c_wal_dir = if let Ok(c) = CString::new(wal_dir.to_string_lossy().as_bytes()) { - c - } else { - return Err(Error::new( - "Failed to convert wal_dir to CString \ - when restoring from latest backup" - .to_owned(), - )); - }; + let c_db_dir = to_cpath(db_dir)?; + let c_wal_dir = to_cpath(wal_dir)?; unsafe { ffi_try!(ffi::rocksdb_backup_engine_restore_db_from_latest_backup( @@ -179,27 +150,8 @@ impl BackupEngine { opts: &RestoreOptions, backup_id: u32, ) -> Result<(), Error> { - let db_dir = db_dir.as_ref(); - let c_db_dir = if let Ok(c) = CString::new(db_dir.to_string_lossy().as_bytes()) { - c - } else { - return Err(Error::new( - "Failed to convert db_dir to CString \ - when restoring from latest backup" - .to_owned(), - )); - }; - - let wal_dir = wal_dir.as_ref(); - let c_wal_dir = if let Ok(c) = CString::new(wal_dir.to_string_lossy().as_bytes()) { - c - } else { - return Err(Error::new( - "Failed to convert wal_dir to CString \ - when restoring from latest backup" - .to_owned(), - )); - }; + let c_db_dir = to_cpath(db_dir)?; + let c_wal_dir = to_cpath(wal_dir)?; unsafe { ffi_try!(ffi::rocksdb_backup_engine_restore_db_from_backup( diff --git a/src/checkpoint.rs b/src/checkpoint.rs index 49bbaef..47459e3 100644 --- a/src/checkpoint.rs +++ b/src/checkpoint.rs @@ -17,8 +17,7 @@ //! //! [1]: https://github.com/facebook/rocksdb/wiki/Checkpoints -use crate::{ffi, Error, DB}; -use std::ffi::CString; +use crate::{ffi, ffi_util::to_cpath, Error, DB}; use std::marker::PhantomData; use std::path::Path; @@ -56,24 +55,15 @@ impl<'db> Checkpoint<'db> { /// Creates new physical DB checkpoint in directory specified by `path`. pub fn create_checkpoint>(&self, path: P) -> Result<(), Error> { - let path = path.as_ref(); - let cpath = if let Ok(c) = CString::new(path.to_string_lossy().as_bytes()) { - c - } else { - return Err(Error::new( - "Failed to convert path to CString when creating DB checkpoint".to_owned(), - )); - }; - + let cpath = to_cpath(path)?; unsafe { ffi_try!(ffi::rocksdb_checkpoint_create( self.inner, cpath.as_ptr(), LOG_SIZE_FOR_FLUSH, )); - - Ok(()) } + Ok(()) } } diff --git a/src/db_options.rs b/src/db_options.rs index 38608e1..276f0cf 100644 --- a/src/db_options.rs +++ b/src/db_options.rs @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::ffi::{CStr, CString}; +use std::ffi::CStr; use std::path::Path; use std::sync::Arc; @@ -24,7 +24,7 @@ use crate::{ comparator::{self, ComparatorCallback, CompareFn}, db::DBAccess, ffi, - ffi_util::CStrLike, + ffi_util::{to_cpath, CStrLike}, merge_operator::{ self, full_merge_callback, partial_merge_callback, MergeFn, MergeOperatorCallback, }, @@ -1535,7 +1535,7 @@ impl Options { /// /// Default: empty pub fn set_db_log_dir>(&mut self, path: P) { - let p = CString::new(path.as_ref().to_string_lossy().as_bytes()).unwrap(); + let p = to_cpath(path).unwrap(); unsafe { ffi::rocksdb_options_set_db_log_dir(self.inner, p.as_ptr()); } @@ -2723,7 +2723,7 @@ impl Options { /// opts.set_wal_dir("/path/to/dir"); /// ``` pub fn set_wal_dir>(&mut self, path: P) { - let p = CString::new(path.as_ref().to_string_lossy().as_bytes()).unwrap(); + let p = to_cpath(path).unwrap(); unsafe { ffi::rocksdb_options_set_wal_dir(self.inner, p.as_ptr()); } @@ -3860,12 +3860,12 @@ pub struct DBPath { impl DBPath { /// Create a new path pub fn new>(path: P, target_size: u64) -> Result { - let p = CString::new(path.as_ref().to_string_lossy().as_bytes()).unwrap(); + let p = to_cpath(path.as_ref()).unwrap(); let dbpath = unsafe { ffi::rocksdb_dbpath_create(p.as_ptr(), target_size) }; if dbpath.is_null() { Err(Error::new(format!( "Could not create path for storing sst files at location: {}", - path.as_ref().to_string_lossy() + path.as_ref().display() ))) } else { Ok(DBPath { inner: dbpath })