Consistently use ffi_util::to_cpath to convert Path to CString (tikv#640)

Author: mina86

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<P: AsRef<Path>>(opts: &BackupEngineOptions, path: P) -> Result<Self, Error> {
-        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(

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<P: AsRef<Path>>(&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(())
     }
 }
 

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<P: AsRef<Path>>(&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<P: AsRef<Path>>(&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<P: AsRef<Path>>(path: P, target_size: u64) -> Result<Self, Error> {
-        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 })