diff --git a/CHANGES.md b/CHANGES.md index 396a458f..796d39e9 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -2,6 +2,10 @@ ## Unreleased +- **Breaking**: Changed a number of APIs using `isize` when `usize` is semantically more appropriate: `Driver::create.*`, `Rasterband::overview`, `Dataset::{layer|into_layer|layer_count}`. + + - + - Created `enum AxisMappingStrategy` for `OSRAxisMappingStrategy` ordinals. - **Breaking**: `SpatialRef::{set_}axis_mapping_strategy` use `AxisMappingStrategy` instead of `gdal_sys::OSRAxisMappingStrategy::Type`. diff --git a/src/driver.rs b/src/driver.rs index c17f46b9..83630ca0 100644 --- a/src/driver.rs +++ b/src/driver.rs @@ -3,7 +3,6 @@ use std::path::Path; use std::sync::Once; use gdal_sys::{self, CPLErr, GDALDriverH, GDALMajorObjectH}; -use libc::c_int; use crate::cpl::CslStringList; use crate::dataset::Dataset; @@ -105,9 +104,9 @@ impl Driver { pub fn create>( &self, filename: P, - size_x: isize, - size_y: isize, - bands: isize, + size_x: usize, + size_y: usize, + bands: usize, ) -> Result { self.create_with_band_type::(filename, size_x, size_y, bands) } @@ -134,9 +133,9 @@ impl Driver { pub fn create_with_band_type>( &self, filename: P, - size_x: isize, - size_y: isize, - bands: isize, + size_x: usize, + size_y: usize, + bands: usize, ) -> Result { let options = []; self.create_with_band_type_with_options::(filename, size_x, size_y, bands, &options) @@ -176,9 +175,9 @@ impl Driver { pub fn create_with_band_type_with_options>( &self, filename: P, - size_x: isize, - size_y: isize, - bands: isize, + size_x: usize, + size_y: usize, + bands: usize, options: &[RasterCreationOption], ) -> Result { Self::_create_with_band_type_with_options( @@ -195,9 +194,9 @@ impl Driver { fn _create_with_band_type_with_options( &self, filename: &Path, - size_x: isize, - size_y: isize, - bands: isize, + size_x: usize, + size_y: usize, + bands: usize, data_type: GdalDataType, options: &[RasterCreationOption], ) -> Result { @@ -206,14 +205,18 @@ impl Driver { options_c.set_name_value(option.key, option.value)?; } + let size_x = libc::c_int::try_from(size_x)?; + let size_y = libc::c_int::try_from(size_y)?; + let bands = libc::c_int::try_from(bands)?; + let c_filename = _path_to_c_string(filename)?; let c_dataset = unsafe { gdal_sys::GDALCreate( self.c_driver, c_filename.as_ptr(), - size_x as c_int, - size_y as c_int, - bands as c_int, + size_x, + size_y, + bands, data_type as u32, options_c.as_ptr(), ) diff --git a/src/errors.rs b/src/errors.rs index 47fd16c3..0525fe58 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -1,6 +1,7 @@ //! GDAL Error Types use libc::c_int; +use std::num::TryFromIntError; use thiserror::Error; use gdal_sys::{CPLErr, OGRErr, OGRFieldType, OGRwkbGeometryType}; @@ -82,6 +83,10 @@ pub enum GdalError { data_type: crate::raster::ExtendedDataTypeClass, method_name: &'static str, }, + #[error(transparent)] + IntConversionError(#[from] TryFromIntError), + #[error("Buffer length {0} does not match raster size {1:?}")] + BufferSizeMismatch(usize, (usize, usize)), } /// A wrapper for [`CPLErr::Type`] that reflects it as an enum diff --git a/src/raster/rasterband.rs b/src/raster/rasterband.rs index c3b462e1..6e09420f 100644 --- a/src/raster/rasterband.rs +++ b/src/raster/rasterband.rs @@ -32,14 +32,11 @@ impl Dataset { /// /// # Errors /// Returns an error if the band cannot be read, including in the case the index is 0. - /// - /// # Panics - /// Panics if the band index is greater than `c_int::MAX`. pub fn rasterband(&self, band_index: usize) -> Result { - assert!(band_index <= c_int::MAX as usize); + let band_index = libc::c_int::try_from(band_index)?; 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); if c_band.is_null() { return Err(_last_null_pointer_err("GDALGetRasterBand")); } @@ -375,7 +372,9 @@ impl<'a> RasterBand<'a> { e_resample_alg: Option, ) -> Result<()> { let pixels = size.0 * size.1; - assert_eq!(buffer.len(), pixels); + if buffer.len() != pixels { + return Err(GdalError::BufferSizeMismatch(buffer.len(), size)); + } let resample_alg = e_resample_alg.unwrap_or(ResampleAlg::NearestNeighbour); @@ -673,14 +672,19 @@ impl<'a> RasterBand<'a> { /// * `window` - the window position from top left /// * `window_size` - the window size (GDAL will interpolate data if window_size != Buffer.size) /// * `buffer` - the data to write into the window - /// pub fn write( &mut self, window: (isize, isize), window_size: (usize, usize), buffer: &Buffer, ) -> Result<()> { - assert_eq!(buffer.data.len(), buffer.size.0 * buffer.size.1); + if buffer.data.len() != buffer.size.0 * buffer.size.1 { + return Err(GdalError::BufferSizeMismatch( + buffer.data.len(), + buffer.size, + )); + } + let rv = unsafe { gdal_sys::GDALRasterIO( self.c_rasterband, @@ -835,10 +839,12 @@ impl<'a> RasterBand<'a> { unsafe { Ok(gdal_sys::GDALGetOverviewCount(self.c_rasterband)) } } - pub fn overview(&self, overview_index: isize) -> Result> { + pub fn overview(&self, overview_index: usize) -> Result> { + let overview_index = libc::c_int::try_from(overview_index)?; + unsafe { let c_band = self.c_rasterband; - let overview = gdal_sys::GDALGetOverview(c_band, overview_index as libc::c_int); + let overview = gdal_sys::GDALGetOverview(c_band, overview_index); if overview.is_null() { return Err(_last_null_pointer_err("GDALGetOverview")); } @@ -1002,21 +1008,11 @@ impl<'a> RasterBand<'a> { /// * `min` - Histogram lower bound /// * `max` - Histogram upper bound /// * `counts` - Histogram values for each bucket - /// - /// # Panics - /// Panics if the `counts.len()` is greater than `i32::MAX`. pub fn set_default_histogram(&self, min: f64, max: f64, counts: &mut [u64]) -> Result<()> { - let n_buckets = counts.len(); - assert!(n_buckets <= i32::MAX as usize); + let n_buckets = libc::c_int::try_from(counts.len())?; let rv = unsafe { - GDALSetDefaultHistogramEx( - self.c_rasterband, - min, - max, - n_buckets as i32, - counts.as_mut_ptr(), - ) + GDALSetDefaultHistogramEx(self.c_rasterband, min, max, n_buckets, counts.as_mut_ptr()) }; match CplErrType::from(rv) { @@ -1048,16 +1044,15 @@ impl<'a> RasterBand<'a> { )); } - assert!(n_buckets <= i32::MAX as usize); - - let mut counts = vec![0; n_buckets]; + let n_buckets = libc::c_int::try_from(n_buckets)?; + let mut counts = vec![0; n_buckets as usize]; let rv = unsafe { GDALGetRasterHistogramEx( self.c_rasterband, min, max, - n_buckets as i32, + n_buckets, counts.as_mut_ptr(), libc::c_int::from(include_out_of_range), libc::c_int::from(is_approx_ok), diff --git a/src/vector/layer.rs b/src/vector/layer.rs index 07a50cfd..43b68ce5 100644 --- a/src/vector/layer.rs +++ b/src/vector/layer.rs @@ -9,7 +9,7 @@ use libc::c_int; use std::ffi::NulError; use std::mem::MaybeUninit; use std::ptr::null_mut; -use std::{convert::TryInto, ffi::CString, marker::PhantomData}; +use std::{ffi::CString, marker::PhantomData}; use crate::errors::*; use crate::vector::feature::{FeatureIterator, OwnedFeatureIterator}; @@ -507,8 +507,8 @@ pub trait LayerAccess: Sized { pub struct LayerIterator<'a> { dataset: &'a Dataset, - idx: isize, - count: isize, + idx: usize, + count: usize, } impl<'a> Iterator for LayerIterator<'a> { @@ -530,10 +530,8 @@ impl<'a> Iterator for LayerIterator<'a> { } fn size_hint(&self) -> (usize, Option) { - match Some(self.count).and_then(|s| s.try_into().ok()) { - Some(size) => (size, Some(size)), - None => (0, None), - } + let size = self.count; + (size, Some(size)) } } @@ -600,16 +598,17 @@ 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 + pub fn layer_count(&self) -> usize { + (unsafe { gdal_sys::OGR_DS_GetLayerCount(self.c_dataset()) }) as usize } /// Fetch a layer by index. /// /// 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) }; + pub fn layer(&self, idx: usize) -> Result { + let idx = libc::c_int::try_from(idx)?; + let c_layer = unsafe { gdal_sys::OGR_DS_GetLayer(self.c_dataset(), idx) }; if c_layer.is_null() { return Err(_last_null_pointer_err("OGR_DS_GetLayer")); } @@ -620,8 +619,9 @@ 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) }; + pub fn into_layer(self, idx: usize) -> Result { + let idx = libc::c_int::try_from(idx)?; + let c_layer = unsafe { gdal_sys::OGR_DS_GetLayer(self.c_dataset(), idx) }; if c_layer.is_null() { return Err(_last_null_pointer_err("OGR_DS_GetLayer")); }