Skip to content

Commit

Permalink
Use ManuallyDrop in Dataset
Browse files Browse the repository at this point in the history
  • Loading branch information
@lnicola
lnicola committed Aug 2, 2023
1 parent 59d7f6e commit e181682
Showing 1 changed file with 54 additions and 33 deletions.
87 changes: 54 additions & 33 deletions src/dataset.rs
View file
Original file line number Diff line number Diff line change
@@ -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},
Expand Down Expand Up @@ -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]
Expand All @@ -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<DatasetPtr>,
closed: bool,
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
})
}
Expand All @@ -414,15 +429,15 @@ 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));
}
}
#[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(())
Expand All @@ -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(())
Expand All @@ -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<SpatialRef> {
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));
}
Expand Down Expand Up @@ -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,
Expand All @@ -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)
}
}
Expand All @@ -546,7 +566,7 @@ impl Dataset {
/// rasterband at the given _1-based_ index.
pub fn rasterband(&self, band_index: isize) -> Result<RasterBand> {
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"));
}
Expand Down Expand Up @@ -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,
Expand All @@ -614,15 +634,15 @@ 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.
///
/// Applies to vector datasets, and fetches by the given
/// _0-based_ index.
pub fn layer(&self, idx: isize) -> Result<Layer> {
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"));
}
Expand All @@ -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<OwnedLayer> {
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"));
}
Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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));
Expand All @@ -787,7 +807,7 @@ impl Dataset {
pub fn geo_transform(&self) -> Result<GeoTransform> {
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 {
Expand Down Expand Up @@ -865,7 +885,7 @@ impl Dataset {
/// ```
pub fn start_transaction(&mut self) -> Result<Transaction<'_>> {
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,
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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()
}
}

Expand All @@ -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);
}
}
}
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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()) };
}
}
}
Expand Down

0 comments on commit e181682

Please sign in to comment.