Merge #413

413: Tweak `RasterBand::actual_block_size` API and replace asserts with `try_into` r=jdroenner a=lnicola

- [x] I agree to follow the project's [code of conduct](https://github.com/georust/gdal/blob/master/CODE_OF_CONDUCT.md).
- [x] I added an entry to `CHANGES.md` if knowledge of this change could be valuable to users.
---



Co-authored-by: Laurențiu Nicola <[email protected]>

CHANGES.md

@@ -1,6 +1,10 @@
 # Changes
 
 ## Unreleased
+- **Breaking**: `RasterBand::actual_block_size` now takes two `usize` offsets instead of `(isize, isize)`
+
+ - <https://github.com/georust/gdal/pull/413>
+
 - Added `GDT_Int8` support
 
   - <https://github.com/georust/gdal/pull/412>

src/gcp.rs

@@ -159,10 +159,10 @@ impl Dataset {
     ///
     /// Panics if `gcps` has more than [`libc::c_int::MAX`] elements.
     pub fn set_gcps(&self, gcps: Vec<Gcp>, spatial_ref: &SpatialRef) -> Result<()> {
-        assert!(
-            gcps.len() <= libc::c_int::MAX as usize,
-            "only up to `INT_MAX` GCPs are supported"
-        );
+        let len = gcps
+            .len()
+            .try_into()
+            .expect("only up to `INT_MAX` GCPs are supported");
 
         struct CGcp {
             id: CString,
@@ -205,7 +205,7 @@ impl Dataset {
         let rv = unsafe {
             gdal_sys::GDALSetGCPs2(
                 self.c_dataset(),
-                gdal_gcps.len() as libc::c_int,
+                len,
                 gdal_gcps.as_ptr(),
                 spatial_ref.to_c_hsrs() as *mut _,
             )

src/raster/rasterband.rs

@@ -581,14 +581,16 @@ impl<'a> RasterBand<'a> {
     /// Get actual block size (at the edges) when block size
     /// does not divide band size.
     #[cfg(any(all(major_is_2, minor_ge_2), major_ge_3))] // GDAL 2.2 .. 2.x or >= 3
-    pub fn actual_block_size(&self, offset: (isize, isize)) -> Result<(usize, usize)> {
+    pub fn actual_block_size(&self, x: usize, y: usize) -> Result<(usize, usize)> {
+        let offset_x = x.try_into().expect("`x` offset must fit in `c_int`");
+        let offset_y = y.try_into().expect("`y` offset must fit in `c_int`");
         let mut block_size_x = 0;
         let mut block_size_y = 0;
         let rv = unsafe {
             gdal_sys::GDALGetActualBlockSize(
                 self.c_rasterband,
-                offset.0 as libc::c_int,
-                offset.1 as libc::c_int,
+                offset_x,
+                offset_y,
                 &mut block_size_x,
                 &mut block_size_y,
             )

src/raster/tests.rs

@@ -515,7 +515,7 @@ fn test_get_rasterband_block_size() {
 fn test_get_rasterband_actual_block_size() {
     let dataset = Dataset::open(fixture("tinymarble.png")).unwrap();
     let rasterband = dataset.rasterband(1).unwrap();
-    let size = rasterband.actual_block_size((0, 0)).unwrap();
+    let size = rasterband.actual_block_size(0, 0).unwrap();
     #[cfg(any(all(major_ge_3, minor_ge_7), major_ge_4))]
     assert_eq!(size, (100, 50));
     #[cfg(not(any(all(major_is_3, minor_ge_7), major_ge_4)))]

src/spatial_ref/srs.rs

@@ -568,19 +568,14 @@ impl SpatialRef {
     ///
     /// Panics if `child` is greater than [`libc::c_int::MAX`].
     pub fn get_attr_value(&self, node_path: &str, child: usize) -> Result<Option<String>> {
-        assert!(
-            child <= libc::c_int::MAX as usize,
-            "`child` must fit in `int`"
-        );
+        let child = child.try_into().expect("`child` must fit in `c_int`");
 
         let c_node_path = CString::new(node_path)?;
-        let c_ptr_value = unsafe {
-            gdal_sys::OSRGetAttrValue(self.0, c_node_path.as_ptr(), child as libc::c_int)
-        };
-        if c_ptr_value.is_null() {
+        let rv = unsafe { gdal_sys::OSRGetAttrValue(self.0, c_node_path.as_ptr(), child) };
+        if rv.is_null() {
             Ok(None)
         } else {
-            Ok(Some(_string(c_ptr_value)))
+            Ok(Some(_string(rv)))
         }
     }