Filenames now must allow references to Path

CHANGES.md

@@ -14,6 +14,9 @@
 - Added `rename` and `delete` to `gdal::Driver`
   - <https://github.com/georust/gdal/pull/226>
 
+- **Breaking**: File paths must now implement `AsRef<Path>`
+  - <https://github.com/georust/gdal/pull/230>
+
 ## 0.8 - 0.10
 
 - Update types to fix build on ppc64le.

src/dataset.rs

@@ -168,15 +168,19 @@ impl Dataset {
 
     /// Open a dataset at the given `path` with default
     /// options.
-    pub fn open(path: &Path) -> Result<Dataset> {
-        Self::open_ex(path, DatasetOptions::default())
+    pub fn open<P: AsRef<Path>>(path: P) -> Result<Dataset> {
+        Self::_open_ex(path.as_ref(), DatasetOptions::default())
     }
 
     /// Open a dataset with extended options. See
     /// [`GDALOpenEx`].
     ///
     /// [`GDALOpenEx`]: https://gdal.org/doxygen/gdal_8h.html#a9cb8585d0b3c16726b08e25bcc94274a
-    pub fn open_ex(path: &Path, options: DatasetOptions) -> Result<Dataset> {
+    pub fn open_ex<P: AsRef<Path>>(path: P, options: DatasetOptions) -> Result<Dataset> {
+        Self::_open_ex(path.as_ref(), options)
+    }
+
+    fn _open_ex(path: &Path, options: DatasetOptions) -> Result<Dataset> {
         crate::driver::_register_drivers();
 
         let c_filename = _path_to_c_string(path)?;
@@ -303,13 +307,22 @@ impl Dataset {
         Ok(())
     }
 
-    pub fn create_copy(
+    pub fn create_copy<P: AsRef<Path>>(
+        &self,
+        driver: &Driver,
+        filename: P,
+        options: &[RasterCreationOption],
+    ) -> Result<Dataset> {
+        Self::_create_copy(&self, driver, filename.as_ref(), options)
+    }
+
+    fn _create_copy(
         &self,
         driver: &Driver,
-        filename: &str,
+        filename: &Path,
         options: &[RasterCreationOption],
     ) -> Result<Dataset> {
-        let c_filename = CString::new(filename)?;
+        let c_filename = _path_to_c_string(filename)?;
 
         let mut c_options = CslStringList::new();
         for option in options {

src/driver.rs

@@ -66,30 +66,48 @@ impl Driver {
         _string(rv)
     }
 
-    pub fn create(
+    pub fn create<P: AsRef<Path>>(
         &self,
-        filename: &str,
+        filename: P,
         size_x: isize,
         size_y: isize,
         bands: isize,
     ) -> Result<Dataset> {
-        self.create_with_band_type::<u8>(filename, size_x, size_y, bands)
+        self.create_with_band_type::<u8, _>(filename, size_x, size_y, bands)
     }
 
-    pub fn create_with_band_type<T: GdalType>(
+    pub fn create_with_band_type<T: GdalType, P: AsRef<Path>>(
         &self,
-        filename: &str,
+        filename: P,
         size_x: isize,
         size_y: isize,
         bands: isize,
     ) -> Result<Dataset> {
         let options = [];
-        self.create_with_band_type_with_options::<T>(filename, size_x, size_y, bands, &options)
+        self.create_with_band_type_with_options::<T, _>(filename, size_x, size_y, bands, &options)
     }
 
-    pub fn create_with_band_type_with_options<T: GdalType>(
+    pub fn create_with_band_type_with_options<T: GdalType, P: AsRef<Path>>(
         &self,
-        filename: &str,
+        filename: P,
+        size_x: isize,
+        size_y: isize,
+        bands: isize,
+        options: &[RasterCreationOption],
+    ) -> Result<Dataset> {
+        Self::_create_with_band_type_with_options::<T>(
+            &self,
+            filename.as_ref(),
+            size_x,
+            size_y,
+            bands,
+            options,
+        )
+    }
+
+    fn _create_with_band_type_with_options<T: GdalType>(
+        &self,
+        filename: &Path,
         size_x: isize,
         size_y: isize,
         bands: isize,
@@ -100,7 +118,7 @@ impl Driver {
             options_c.set_name_value(option.key, option.value)?;
         }
 
-        let c_filename = CString::new(filename)?;
+        let c_filename = _path_to_c_string(filename)?;
         let c_dataset = unsafe {
             gdal_sys::GDALCreate(
                 self.c_driver,
@@ -120,8 +138,8 @@ impl Driver {
         Ok(unsafe { Dataset::from_c_dataset(c_dataset) })
     }
 
-    pub fn create_vector_only(&self, filename: &str) -> Result<Dataset> {
-        self.create_with_band_type::<u8>(filename, 0, 0, 0)
+    pub fn create_vector_only<P: AsRef<Path>>(&self, filename: P) -> Result<Dataset> {
+        self.create_with_band_type::<u8, _>(filename, 0, 0, 0)
     }
 
     /// Delete named dataset.
@@ -130,7 +148,11 @@ impl Driver {
     ///
     /// Calls `GDALDeleteDataset()`
     ///
-    pub fn delete(&self, filename: &Path) -> Result<()> {
+    pub fn delete<P: AsRef<Path>>(&self, filename: P) -> Result<()> {
+        Self::_delete(&self, filename.as_ref())
+    }
+
+    fn _delete(&self, filename: &Path) -> Result<()> {
         let c_filename = _path_to_c_string(filename)?;
 
         let rv = unsafe { gdal_sys::GDALDeleteDataset(self.c_driver, c_filename.as_ptr()) };
@@ -148,7 +170,15 @@ impl Driver {
     ///
     /// Calls `GDALRenameDataset()`
     ///
-    pub fn rename(&self, new_filename: &Path, old_filename: &Path) -> Result<()> {
+    pub fn rename<P1: AsRef<Path>, P2: AsRef<Path>>(
+        &self,
+        new_filename: P1,
+        old_filename: P2,
+    ) -> Result<()> {
+        Self::_rename(&self, new_filename.as_ref(), old_filename.as_ref())
+    }
+
+    fn _rename(&self, new_filename: &Path, old_filename: &Path) -> Result<()> {
         let c_old_filename = _path_to_c_string(old_filename)?;
         let c_new_filename = _path_to_c_string(new_filename)?;
 

src/raster/tests.rs

@@ -155,9 +155,7 @@ fn test_rename_remove_raster() {
 
     let driver = Driver::get("GTiff").unwrap();
 
-    dataset
-        .create_copy(&driver, &mem_file_path_a.to_string_lossy(), &[])
-        .unwrap();
+    dataset.create_copy(&driver, &mem_file_path_a, &[]).unwrap();
 
     driver.rename(mem_file_path_b, mem_file_path_a).unwrap();
 
@@ -270,7 +268,9 @@ fn test_create() {
 #[test]
 fn test_create_with_band_type() {
     let driver = Driver::get("MEM").unwrap();
-    let dataset = driver.create_with_band_type::<f32>("", 10, 20, 3).unwrap();
+    let dataset = driver
+        .create_with_band_type::<f32, _>("", 10, 20, 3)
+        .unwrap();
     assert_eq!(dataset.raster_size(), (10, 20));
     assert_eq!(dataset.raster_count(), 3);
     assert_eq!(dataset.driver().short_name(), "MEM");
@@ -307,7 +307,7 @@ fn test_create_with_band_type_with_options() {
     let tmp_filename = "/tmp/test.tif";
     {
         let dataset = driver
-            .create_with_band_type_with_options::<u8>(tmp_filename, 256, 256, 1, &options)
+            .create_with_band_type_with_options::<u8, _>(tmp_filename, 256, 256, 1, &options)
             .unwrap();
         let rasterband = dataset.rasterband(1).unwrap();
         let block_size = rasterband.block_size();

src/vector/vector_tests/mod.rs

@@ -631,7 +631,7 @@ mod tests {
         {
             let driver = Driver::get("GeoJSON").unwrap();
             let mut ds = driver
-                .create_vector_only(&fixture!("output.geojson").to_string_lossy())
+                .create_vector_only(&fixture!("output.geojson"))
                 .unwrap();
             let mut layer = ds.create_layer(Default::default()).unwrap();
             layer

src/vsi.rs

@@ -1,15 +1,19 @@
-use std::ffi::CString;
 use std::marker::PhantomData;
 use std::mem::ManuallyDrop;
+use std::path::{Path, PathBuf};
 
 use gdal_sys::{VSIFCloseL, VSIFileFromMemBuffer, VSIFree, VSIGetMemFileBuffer, VSIUnlink};
 
 use crate::errors::{GdalError, Result};
-use crate::utils::_last_null_pointer_err;
+use crate::utils::{_last_null_pointer_err, _path_to_c_string};
 
 /// Creates a new VSIMemFile from a given buffer.
-pub fn create_mem_file(file_name: &str, data: Vec<u8>) -> Result<()> {
-    let file_name = CString::new(file_name)?;
+pub fn create_mem_file<P: AsRef<Path>>(file_name: P, data: Vec<u8>) -> Result<()> {
+    _create_mem_file(file_name.as_ref(), data)
+}
+
+fn _create_mem_file(file_name: &Path, data: Vec<u8>) -> Result<()> {
+    let file_name = _path_to_c_string(file_name)?;
 
     // ownership will be given to GDAL, so it should not be automaticly dropped
     let mut data = ManuallyDrop::new(data);
@@ -39,14 +43,14 @@ pub fn create_mem_file(file_name: &str, data: Vec<u8>) -> Result<()> {
 /// A helper struct that unlinks a mem file that points to borrowed data
 /// before that data is freed.
 pub struct MemFileRef<'d> {
-    file_name: String,
+    file_name: PathBuf,
     data_ref: PhantomData<&'d mut ()>,
 }
 
 impl<'d> MemFileRef<'d> {
-    pub fn new(file_name: &str) -> MemFileRef<'d> {
+    pub fn new(file_name: &Path) -> MemFileRef<'d> {
         Self {
-            file_name: file_name.to_string(),
+            file_name: file_name.into(),
             data_ref: PhantomData::default(),
         }
     }
@@ -62,8 +66,15 @@ impl<'d> Drop for MemFileRef<'d> {
 
 /// Creates a new VSIMemFile from a given buffer reference.
 /// Returns a handle that has a lifetime that is shorter than `data`.
-pub fn create_mem_file_from_ref<'d>(file_name: &str, data: &'d mut [u8]) -> Result<MemFileRef<'d>> {
-    let file_name_c = CString::new(file_name)?;
+pub fn create_mem_file_from_ref<P: AsRef<Path>>(
+    file_name: P,
+    data: &mut [u8],
+) -> Result<MemFileRef<'_>> {
+    _create_mem_file_from_ref(file_name.as_ref(), data)
+}
+
+fn _create_mem_file_from_ref<'d>(file_name: &Path, data: &'d mut [u8]) -> Result<MemFileRef<'d>> {
+    let file_name_c = _path_to_c_string(&file_name)?;
 
     let handle = unsafe {
         VSIFileFromMemBuffer(
@@ -86,14 +97,18 @@ pub fn create_mem_file_from_ref<'d>(file_name: &str, data: &'d mut [u8]) -> Resu
 }
 
 /// Unlink a VSIMemFile.
-pub fn unlink_mem_file(file_name: &str) -> Result<()> {
-    let file_name_c = CString::new(file_name)?;
+pub fn unlink_mem_file<P: AsRef<Path>>(file_name: P) -> Result<()> {
+    _unlink_mem_file(file_name.as_ref())
+}
+
+fn _unlink_mem_file(file_name: &Path) -> Result<()> {
+    let file_name_c = _path_to_c_string(&file_name)?;
 
     let rv = unsafe { VSIUnlink(file_name_c.as_ptr()) };
 
     if rv != 0 {
         return Err(GdalError::UnlinkMemFile {
-            file_name: file_name.to_string(),
+            file_name: file_name.display().to_string(),
         });
     }
 
@@ -102,8 +117,12 @@ pub fn unlink_mem_file(file_name: &str) -> Result<()> {
 
 /// Copies the bytes of the VSIMemFile with given `file_name`.
 /// Takes the ownership and frees the memory of the VSIMemFile.
-pub fn get_vsi_mem_file_bytes_owned(file_name: &str) -> Result<Vec<u8>> {
-    let file_name = CString::new(file_name)?;
+pub fn get_vsi_mem_file_bytes_owned<P: AsRef<Path>>(file_name: P) -> Result<Vec<u8>> {
+    _get_vsi_mem_file_bytes_owned(file_name.as_ref())
+}
+
+fn _get_vsi_mem_file_bytes_owned(file_name: &Path) -> Result<Vec<u8>> {
+    let file_name = _path_to_c_string(file_name)?;
 
     let owned_bytes = unsafe {
         let mut length: u64 = 0;
@@ -126,11 +145,18 @@ pub fn get_vsi_mem_file_bytes_owned(file_name: &str) -> Result<Vec<u8>> {
 
 /// Computes a function on the bytes of the vsi in-memory file with given `file_name`.
 /// This method is useful if you don't want to take the ownership of the memory.
-pub fn call_on_mem_file_bytes<F, R>(file_name: &str, fun: F) -> Result<R>
+pub fn call_on_mem_file_bytes<F, R, P: AsRef<Path>>(file_name: P, fun: F) -> Result<R>
+where
+    F: FnOnce(&[u8]) -> R,
+{
+    _call_on_mem_file_bytes(file_name.as_ref(), fun)
+}
+
+fn _call_on_mem_file_bytes<F, R>(file_name: &Path, fun: F) -> Result<R>
 where
     F: FnOnce(&[u8]) -> R,
 {
-    let file_name = CString::new(file_name)?;
+    let file_name = _path_to_c_string(file_name)?;
 
     unsafe {
         let mut length: u64 = 0;