A Consistent and Principled Approach to GDAL C Pointers

.cargo/config.toml

@@ -1,5 +1,5 @@
 [alias]
-# Run doctests, displaying compiler output
+# doc-test-output: Run doctests, displaying compiler output
 dto = "test --doc -- --show-output"
 # Run clippy, raising warnings to errors
 nowarn = "clippy --all-targets -- -D warnings"

Cargo.toml

@@ -26,6 +26,7 @@ ndarray = { version = "0.15", optional = true }
 chrono = { version = "0.4.26", default-features = false }
 bitflags = "2.4"
 once_cell = "1.18"
+foreign-types = "0.5.0"
 
 [build-dependencies]
 semver = "1.0"

examples/rasterband.rs

@@ -1,4 +1,3 @@
-use gdal::raster::RasterBand;
 use gdal::{Dataset, Metadata};
 use std::path::Path;
 
@@ -7,7 +6,7 @@ fn main() {
     let dataset = Dataset::open(path).unwrap();
     println!("dataset description: {:?}", dataset.description());
 
-    let rasterband: RasterBand = dataset.rasterband(1).unwrap();
+    let rasterband = dataset.rasterband(1).unwrap();
     println!("rasterband description: {:?}", rasterband.description());
     println!("rasterband no_data_value: {:?}", rasterband.no_data_value());
     println!("rasterband type: {:?}", rasterband.band_type());

examples/read_write_ogr_datetime.rs

@@ -32,7 +32,7 @@ fn run() -> gdal::errors::Result<()> {
     for feature_a in layer_a.features() {
         let mut ft = Feature::new(&defn)?;
         if let Some(geom) = feature_a.geometry() {
-            ft.set_geometry(geom.clone())?;
+            ft.set_geometry(geom.to_owned())?;
         }
         // copy each field value of the feature:
         for field in defn.fields() {

fixtures/tinymarble.tif.aux.xml

@@ -1,3 +1,12 @@
 <PAMDataset>
   <SRS dataAxisToSRSAxisMapping="2,1">GEOGCS["WGS 84",DATUM["WGS_1984",SPHEROID["WGS 84",6378137,298.257223563,AUTHORITY["EPSG","7030"]],AUTHORITY["EPSG","6326"]],PRIMEM["Greenwich",0,AUTHORITY["EPSG","8901"]],UNIT["degree",0.0174532925199433,AUTHORITY["EPSG","9122"]],AXIS["Latitude",NORTH],AXIS["Longitude",EAST],AUTHORITY["EPSG","4326"]]</SRS>
+  <PAMRasterBand band="1">
+    <Metadata>
+      <MDI key="STATISTICS_MAXIMUM">255</MDI>
+      <MDI key="STATISTICS_MEAN">68.4716</MDI>
+      <MDI key="STATISTICS_MINIMUM">0</MDI>
+      <MDI key="STATISTICS_STDDEV">83.68444773935</MDI>
+      <MDI key="STATISTICS_VALID_PERCENT">100</MDI>
+    </Metadata>
+  </PAMRasterBand>
 </PAMDataset>

src/dataset.rs

@@ -1,3 +1,4 @@
+use foreign_types::ForeignTypeRef;
 use std::{ffi::CString, ffi::NulError, path::Path, ptr};
 
 use gdal_sys::{self, CPLErr, GDALDatasetH, GDALMajorObjectH};
@@ -6,6 +7,7 @@ use crate::cpl::CslStringList;
 use crate::errors::*;
 use crate::options::DatasetOptions;
 use crate::raster::RasterCreationOption;
+use crate::spatial_ref::SpatialRefRef;
 use crate::utils::{_last_cpl_err, _last_null_pointer_err, _path_to_c_string, _string};
 use crate::{
     gdal_major_object::MajorObject, spatial_ref::SpatialRef, Driver, GeoTransform, Metadata,
@@ -224,13 +226,16 @@ impl Dataset {
     #[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)) }
+        Ok(
+            unsafe { SpatialRefRef::from_ptr(gdal_sys::GDALGetSpatialRef(self.c_dataset)) }
+                .to_owned(),
+        )
     }
 
     #[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.as_ptr()) };
         if rv != CPLErr::CE_None {
             return Err(_last_cpl_err(rv));
         }

src/gcp.rs

@@ -6,9 +6,10 @@ use std::marker::PhantomData;
 use gdal_sys::CPLErr;
 
 use crate::errors::Result;
-use crate::spatial_ref::SpatialRef;
+use crate::spatial_ref::{SpatialRef, SpatialRefRef};
 use crate::utils::{_last_cpl_err, _string};
 use crate::Dataset;
+use foreign_types::{ForeignType, ForeignTypeRef};
 
 /// An owned Ground Control Point.
 #[derive(Clone, Debug, PartialEq, PartialOrd)]
@@ -120,8 +121,9 @@ impl Dataset {
         if c_ptr.is_null() {
             return None;
         }
-
-        unsafe { SpatialRef::from_c_obj(c_ptr) }.ok()
+        // NB: Creating a `SpatialRefRef` from a pointer acknowledges that GDAL is giving us
+        // a shared reference. We then call `to_owned` to appropriately get a clone.
+        Some(unsafe { SpatialRefRef::from_ptr(c_ptr) }.to_owned())
     }
 
     /// Get the projection definition string for the GCPs in this dataset.
@@ -207,7 +209,7 @@ impl Dataset {
                 self.c_dataset(),
                 len,
                 gdal_gcps.as_ptr(),
-                spatial_ref.to_c_hsrs() as *mut _,
+                spatial_ref.as_ptr() as *mut _,
             )
         };
 

src/metadata.rs

@@ -382,7 +382,7 @@ mod tests {
     fn test_set_description() {
         let driver = DriverManager::get_driver_by_name("MEM").unwrap();
         let dataset = driver.create("", 1, 1, 1).unwrap();
-        let mut band = dataset.rasterband(1).unwrap();
+        let band = dataset.rasterband(1).unwrap();
 
         let description = "A merry and cheerful band description";
         assert_eq!(band.description().unwrap(), "");

src/raster/gcp.rs

@@ -0,0 +1,38 @@
+//! Raster ground control point support
+
+use foreign_types::ForeignTypeRef;
+use crate::spatial_ref::SpatialRefRef;
+use crate::Dataset;
+
+impl Dataset {
+    /// Get output spatial reference system for GCPs.
+    ///
+    /// # Notes
+    /// * This is separate and distinct from [`Dataset::spatial_ref`], and only applies to
+    /// the representation of ground control points, when embedded.
+    ///
+    /// See: [`GDALGetGCPSpatialRef`](https://gdal.org/api/raster_c_api.html#_CPPv420GDALGetGCPSpatialRef12GDALDatasetH)
+    pub fn gcp_spatial_ref(&self) -> Option<&SpatialRefRef> {
+        let c_ptr = unsafe { gdal_sys::GDALGetGCPSpatialRef(self.c_dataset()) };
+
+        if c_ptr.is_null() {
+            return None;
+        }
+
+        Some(unsafe { SpatialRefRef::from_ptr(c_ptr) })
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use crate::test_utils::fixture;
+    use crate::Dataset;
+
+    #[test]
+    fn test_gcp_spatial_ref() {
+        let dataset = Dataset::open(fixture("gcp.tif")).unwrap();
+        let gcp_srs = dataset.gcp_spatial_ref();
+        let auth = gcp_srs.and_then(|s| s.authority().ok());
+        assert_eq!(auth.unwrap(), "EPSG:4326");
+    }
+}

src/raster/mdarray.rs

@@ -1,6 +1,6 @@
 use super::GdalType;
 use crate::errors::*;
-use crate::spatial_ref::SpatialRef;
+use crate::spatial_ref::{SpatialRef, SpatialRefRef};
 use crate::utils::{_last_cpl_err, _last_null_pointer_err, _string, _string_array};
 use crate::{cpl::CslStringList, Dataset};
 use gdal_sys::{
@@ -16,12 +16,13 @@ use gdal_sys::{
     GDALGroupOpenGroup, GDALGroupOpenMDArray, GDALGroupRelease, GDALMDArrayGetAttribute,
     GDALMDArrayGetDataType, GDALMDArrayGetDimensionCount, GDALMDArrayGetDimensions,
     GDALMDArrayGetNoDataValueAsDouble, GDALMDArrayGetSpatialRef, GDALMDArrayGetTotalElementsCount,
-    GDALMDArrayGetUnit, GDALMDArrayH, GDALMDArrayRelease, OSRDestroySpatialReference, VSIFree,
+    GDALMDArrayGetUnit, GDALMDArrayH, GDALMDArrayRelease, VSIFree,
 };
 use libc::c_void;
 use std::ffi::CString;
 use std::os::raw::c_char;
 
+use foreign_types::ForeignTypeRef;
 #[cfg(feature = "ndarray")]
 use ndarray::{ArrayD, IxDyn};
 use std::fmt::{Debug, Display};
@@ -324,15 +325,9 @@ impl<'a> MDArray<'a> {
     }
 
     pub fn spatial_reference(&self) -> Result<SpatialRef> {
-        unsafe {
-            let c_gdal_spatial_ref = GDALMDArrayGetSpatialRef(self.c_mdarray);
-
-            let gdal_spatial_ref = SpatialRef::from_c_obj(c_gdal_spatial_ref);
-
-            OSRDestroySpatialReference(c_gdal_spatial_ref);
-
-            gdal_spatial_ref
-        }
+        // NB: Creating a `SpatialRefRef` from a pointer acknowledges that GDAL is giving us
+        // a shared reference. We then call `to_owned` to appropriately get a clone.
+        Ok(unsafe { SpatialRefRef::from_ptr(GDALMDArrayGetSpatialRef(self.c_mdarray)) }.to_owned())
     }
 
     pub fn no_data_value_as_double(&self) -> Option<f64> {