diff --git a/src/dataset.rs b/src/dataset.rs index 1a113986..ef6565c4 100644 --- a/src/dataset.rs +++ b/src/dataset.rs @@ -1,6 +1,6 @@ use ptr::null_mut; use std::convert::TryInto; -use std::mem::MaybeUninit; +use std::mem::{ManuallyDrop, MaybeUninit}; use std::{ ffi::NulError, ffi::{CStr, CString}, @@ -162,6 +162,20 @@ impl GeoTransformEx for GeoTransform { } } +/// Wrapper around the native pointer that calls [`GDALClose`] on drop. +/// +/// We use this with `ManuallyDrop` to implement [`Dataset::close`]. +#[derive(Debug)] +struct DatasetPtr(GDALDatasetH); + +impl Drop for DatasetPtr { + fn drop(&mut self) { + unsafe { + gdal_sys::GDALClose(self.0); + } + } +} + /// Wrapper around a [`GDALDataset`][GDALDataset] object. /// /// Represents both a [vector dataset][vector-data-model] @@ -173,7 +187,7 @@ impl GeoTransformEx for GeoTransform { /// [GDALDataset]: https://gdal.org/api/gdaldataset_cpp.html#_CPPv411GDALDataset #[derive(Debug)] pub struct Dataset { - c_dataset: GDALDatasetH, + dataset: ManuallyDrop, closed: bool, } @@ -296,7 +310,7 @@ impl Dataset { /// # Safety /// This method returns a raw C pointer pub unsafe fn c_dataset(&self) -> GDALDatasetH { - self.c_dataset + self.dataset.0 } /// Open a dataset at the given `path` with default @@ -400,8 +414,9 @@ impl Dataset { if c_dataset.is_null() { return Err(_last_null_pointer_err("GDALOpenEx")); } + let c_dataset = ManuallyDrop::new(DatasetPtr(c_dataset)); Ok(Dataset { - c_dataset, + dataset: c_dataset, closed: false, }) } @@ -414,7 +429,7 @@ impl Dataset { pub fn flush_cache(&mut self) -> Result<()> { #[cfg(any(all(major_ge_3, minor_ge_7), major_ge_4))] { - let rv = unsafe { gdal_sys::GDALFlushCache(self.c_dataset) }; + let rv = unsafe { gdal_sys::GDALFlushCache(self.c_dataset()) }; if rv != CPLErr::CE_None { return Err(_last_cpl_err(rv)); } @@ -422,7 +437,7 @@ impl Dataset { #[cfg(not(any(all(major_is_3, minor_ge_7), major_ge_4)))] { unsafe { - gdal_sys::GDALFlushCache(self.c_dataset); + gdal_sys::GDALFlushCache(self.c_dataset()); } } Ok(()) @@ -438,15 +453,19 @@ impl Dataset { #[cfg(any(all(major_ge_3, minor_ge_7), major_ge_4))] { - let rv = unsafe { gdal_sys::GDALClose(self.c_dataset) }; + let c_dataset = ManuallyDrop::into_inner(self.dataset); + + // SAFETY: we only close the dataset here and in `drop`, and we set and check `closed` + let rv = unsafe { gdal_sys::GDALClose(c_dataset.0) }; if rv != CPLErr::CE_None { return Err(_last_cpl_err(rv)); } } #[cfg(not(any(all(major_is_3, minor_ge_7), major_ge_4)))] { + // SAFETY: we only close the dataset here and in `drop`, and we set and check `closed` unsafe { - gdal_sys::GDALClose(self.c_dataset); + ManuallyDrop::drop(&mut self.dataset); } } Ok(()) @@ -458,35 +477,36 @@ impl Dataset { /// This method operates on a raw C pointer /// The dataset must not have been closed (using [`GDALClose`]) before. pub unsafe fn from_c_dataset(c_dataset: GDALDatasetH) -> Dataset { + let c_dataset = ManuallyDrop::new(DatasetPtr(c_dataset)); Dataset { - c_dataset, + dataset: c_dataset, closed: false, } } /// Fetch the projection definition string for this dataset. pub fn projection(&self) -> String { - let rv = unsafe { gdal_sys::GDALGetProjectionRef(self.c_dataset) }; + let rv = unsafe { gdal_sys::GDALGetProjectionRef(self.c_dataset()) }; _string(rv) } /// Set the projection reference string for this dataset. pub fn set_projection(&mut self, projection: &str) -> Result<()> { let c_projection = CString::new(projection)?; - unsafe { gdal_sys::GDALSetProjection(self.c_dataset, c_projection.as_ptr()) }; + unsafe { gdal_sys::GDALSetProjection(self.c_dataset(), c_projection.as_ptr()) }; Ok(()) } #[cfg(major_ge_3)] /// Get the spatial reference system for this dataset. pub fn spatial_ref(&self) -> Result { - unsafe { SpatialRef::from_c_obj(gdal_sys::GDALGetSpatialRef(self.c_dataset)) } + unsafe { SpatialRef::from_c_obj(gdal_sys::GDALGetSpatialRef(self.c_dataset())) } } #[cfg(major_ge_3)] /// Set the spatial reference system for this dataset. pub fn set_spatial_ref(&mut self, spatial_ref: &SpatialRef) -> Result<()> { - let rv = unsafe { gdal_sys::GDALSetSpatialRef(self.c_dataset, spatial_ref.to_c_hsrs()) }; + let rv = unsafe { gdal_sys::GDALSetSpatialRef(self.c_dataset(), spatial_ref.to_c_hsrs()) }; if rv != CPLErr::CE_None { return Err(_last_cpl_err(rv)); } @@ -519,7 +539,7 @@ impl Dataset { gdal_sys::GDALCreateCopy( driver.c_driver(), c_filename.as_ptr(), - self.c_dataset, + self.c_dataset(), 0, c_options.as_ptr(), None, @@ -535,7 +555,7 @@ impl Dataset { /// Fetch the driver to which this dataset relates. pub fn driver(&self) -> Driver { unsafe { - let c_driver = gdal_sys::GDALGetDatasetDriver(self.c_dataset); + let c_driver = gdal_sys::GDALGetDatasetDriver(self.c_dataset()); Driver::from_c_driver(c_driver) } } @@ -546,7 +566,7 @@ impl Dataset { /// rasterband at the given _1-based_ index. pub fn rasterband(&self, band_index: isize) -> Result { unsafe { - let c_band = gdal_sys::GDALGetRasterBand(self.c_dataset, band_index as c_int); + let c_band = gdal_sys::GDALGetRasterBand(self.c_dataset(), band_index as c_int); if c_band.is_null() { return Err(_last_null_pointer_err("GDALGetRasterBand")); } @@ -588,7 +608,7 @@ impl Dataset { let c_resampling = CString::new(resampling)?; let rv = unsafe { gdal_sys::GDALBuildOverviews( - self.c_dataset, + self.c_dataset(), c_resampling.as_ptr(), overviews.len() as i32, overviews.as_ptr() as *mut i32, @@ -614,7 +634,7 @@ impl Dataset { /// Get the number of layers in this dataset. pub fn layer_count(&self) -> isize { - (unsafe { gdal_sys::OGR_DS_GetLayerCount(self.c_dataset) }) as isize + (unsafe { gdal_sys::OGR_DS_GetLayerCount(self.c_dataset()) }) as isize } /// Fetch a layer by index. @@ -622,7 +642,7 @@ impl Dataset { /// Applies to vector datasets, and fetches by the given /// _0-based_ index. pub fn layer(&self, idx: isize) -> Result { - let c_layer = unsafe { gdal_sys::OGR_DS_GetLayer(self.c_dataset, idx as c_int) }; + let c_layer = unsafe { gdal_sys::OGR_DS_GetLayer(self.c_dataset(), idx as c_int) }; if c_layer.is_null() { return Err(_last_null_pointer_err("OGR_DS_GetLayer")); } @@ -634,7 +654,7 @@ impl Dataset { /// Applies to vector datasets, and fetches by the given /// _0-based_ index. pub fn into_layer(self, idx: isize) -> Result { - let c_layer = unsafe { gdal_sys::OGR_DS_GetLayer(self.c_dataset, idx as c_int) }; + let c_layer = unsafe { gdal_sys::OGR_DS_GetLayer(self.c_dataset(), idx as c_int) }; if c_layer.is_null() { return Err(_last_null_pointer_err("OGR_DS_GetLayer")); } @@ -668,13 +688,13 @@ impl Dataset { /// Fetch the number of raster bands on this dataset. pub fn raster_count(&self) -> isize { - (unsafe { gdal_sys::GDALGetRasterCount(self.c_dataset) }) as isize + (unsafe { gdal_sys::GDALGetRasterCount(self.c_dataset()) }) as isize } /// Returns the raster dimensions: (width, height). pub fn raster_size(&self) -> (usize, usize) { - let size_x = unsafe { gdal_sys::GDALGetRasterXSize(self.c_dataset) } as usize; - let size_y = unsafe { gdal_sys::GDALGetRasterYSize(self.c_dataset) } as usize; + let size_x = unsafe { gdal_sys::GDALGetRasterXSize(self.c_dataset()) } as usize; + let size_y = unsafe { gdal_sys::GDALGetRasterYSize(self.c_dataset()) } as usize; (size_x, size_y) } @@ -738,7 +758,7 @@ impl Dataset { // propagated to the gdal_sys wrapper. The lack of `const` seems like a mistake in the // GDAL API, so we just do a cast here. gdal_sys::OGR_DS_CreateLayer( - self.c_dataset, + self.c_dataset(), c_name.as_ptr(), c_srs, options.ty, @@ -767,7 +787,7 @@ impl Dataset { pub fn set_geo_transform(&mut self, transformation: &GeoTransform) -> Result<()> { assert_eq!(transformation.len(), 6); let rv = unsafe { - gdal_sys::GDALSetGeoTransform(self.c_dataset, transformation.as_ptr() as *mut f64) + gdal_sys::GDALSetGeoTransform(self.c_dataset(), transformation.as_ptr() as *mut f64) }; if rv != CPLErr::CE_None { return Err(_last_cpl_err(rv)); @@ -787,7 +807,7 @@ impl Dataset { pub fn geo_transform(&self) -> Result { let mut transformation = GeoTransform::default(); let rv = - unsafe { gdal_sys::GDALGetGeoTransform(self.c_dataset, transformation.as_mut_ptr()) }; + unsafe { gdal_sys::GDALGetGeoTransform(self.c_dataset(), transformation.as_mut_ptr()) }; // check if the dataset has a GeoTransform if rv != CPLErr::CE_None { @@ -865,7 +885,7 @@ impl Dataset { /// ``` pub fn start_transaction(&mut self) -> Result> { let force = 1; - let rv = unsafe { gdal_sys::GDALDatasetStartTransaction(self.c_dataset, force) }; + let rv = unsafe { gdal_sys::GDALDatasetStartTransaction(self.c_dataset(), force) }; if rv != OGRErr::OGRERR_NONE { return Err(GdalError::OgrError { err: rv, @@ -992,7 +1012,7 @@ impl<'a> Iterator for LayerIterator<'a> { if idx < self.count { self.idx += 1; let c_layer = - unsafe { gdal_sys::OGR_DS_GetLayer(self.dataset.c_dataset, idx as c_int) }; + unsafe { gdal_sys::OGR_DS_GetLayer(self.dataset.c_dataset(), idx as c_int) }; if !c_layer.is_null() { let layer = unsafe { Layer::from_c_layer(self.dataset, c_layer) }; return Some(layer); @@ -1021,7 +1041,7 @@ impl<'a> LayerIterator<'a> { impl MajorObject for Dataset { unsafe fn gdal_object_ptr(&self) -> GDALMajorObjectH { - self.c_dataset + self.c_dataset() } } @@ -1030,8 +1050,9 @@ impl Metadata for Dataset {} impl Drop for Dataset { fn drop(&mut self) { if !self.closed { + // SAFETY: we set `closed` in `[Dataset::close]` unsafe { - gdal_sys::GDALClose(self.c_dataset); + ManuallyDrop::drop(&mut self.dataset); } } } @@ -1080,7 +1101,7 @@ impl<'a> Transaction<'a> { /// /// Depending on drivers, this may or may not abort layer sequential readings that are active. pub fn commit(mut self) -> Result<()> { - let rv = unsafe { gdal_sys::GDALDatasetCommitTransaction(self.dataset.c_dataset) }; + let rv = unsafe { gdal_sys::GDALDatasetCommitTransaction(self.dataset.c_dataset()) }; self.rollback_on_drop = false; if rv != OGRErr::OGRERR_NONE { return Err(GdalError::OgrError { @@ -1095,7 +1116,7 @@ impl<'a> Transaction<'a> { /// /// If the rollback fails, will return [`OGRErr::OGRERR_FAILURE`]. pub fn rollback(mut self) -> Result<()> { - let rv = unsafe { gdal_sys::GDALDatasetRollbackTransaction(self.dataset.c_dataset) }; + let rv = unsafe { gdal_sys::GDALDatasetRollbackTransaction(self.dataset.c_dataset()) }; self.rollback_on_drop = false; if rv != OGRErr::OGRERR_NONE { return Err(GdalError::OgrError { @@ -1126,7 +1147,7 @@ impl<'a> Drop for Transaction<'a> { if self.rollback_on_drop { // We silently swallow any errors, because we have no way to report them from a drop // function apart from panicking. - unsafe { gdal_sys::GDALDatasetRollbackTransaction(self.dataset.c_dataset) }; + unsafe { gdal_sys::GDALDatasetRollbackTransaction(self.dataset.c_dataset()) }; } } }