Copilot commented on code in PR #695: URL: https://github.com/apache/sedona-db/pull/695#discussion_r2932339668
########## c/sedona-gdal/src/spatial_ref.rs: ########## @@ -0,0 +1,156 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//! Ported (and contains copied code) from georust/gdal: +//! <https://github.com/georust/gdal/blob/v0.19.0/src/spatial_ref/srs.rs>. +//! Original code is licensed under MIT. + +use std::ffi::{CStr, CString}; +use std::ptr; + +use crate::errors::{GdalError, Result}; +use crate::gdal_api::{call_gdal_api, GdalApi}; +use crate::gdal_dyn_bindgen::OGRERR_NONE; +use crate::gdal_dyn_bindgen::*; + +/// An OGR spatial reference system. +pub struct SpatialRef { + api: &'static GdalApi, + c_srs: OGRSpatialReferenceH, +} + Review Comment: The local `unsafe impl Send for SpatialRef {}` should be justified with a short safety comment explaining why moving `OGRSpatialReferenceH` across threads is sound in this crate’s usage model (and why it is not `Sync`, if that’s intentional). This makes the unsafety auditable and consistent with other unsafe impls in the crate. ########## c/sedona-gdal/src/spatial_ref.rs: ########## @@ -0,0 +1,156 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//! Ported (and contains copied code) from georust/gdal: +//! <https://github.com/georust/gdal/blob/v0.19.0/src/spatial_ref/srs.rs>. +//! Original code is licensed under MIT. + +use std::ffi::{CStr, CString}; +use std::ptr; + +use crate::errors::{GdalError, Result}; +use crate::gdal_api::{call_gdal_api, GdalApi}; +use crate::gdal_dyn_bindgen::OGRERR_NONE; +use crate::gdal_dyn_bindgen::*; + +/// An OGR spatial reference system. +pub struct SpatialRef { + api: &'static GdalApi, + c_srs: OGRSpatialReferenceH, +} + +unsafe impl Send for SpatialRef {} + +impl Drop for SpatialRef { + fn drop(&mut self) { + if !self.c_srs.is_null() { + unsafe { call_gdal_api!(self.api, OSRRelease, self.c_srs) }; + } + } +} + +impl SpatialRef { + /// Create a new SpatialRef from a WKT string. + pub fn from_wkt(api: &'static GdalApi, wkt: &str) -> Result<Self> { + let c_wkt = CString::new(wkt)?; + let c_srs = unsafe { call_gdal_api!(api, OSRNewSpatialReference, c_wkt.as_ptr()) }; + if c_srs.is_null() { + return Err(GdalError::NullPointer { + method_name: "OSRNewSpatialReference", + msg: "failed to create spatial reference from WKT".to_string(), + }); + } + Ok(Self { api, c_srs }) + } + + /// Create a SpatialRef by cloning a borrowed C handle via `OSRClone`. + /// + /// # Safety + /// + /// The caller must ensure `c_srs` is a valid `OGRSpatialReferenceH`. + pub unsafe fn from_c_srs_clone( + api: &'static GdalApi, + c_srs: OGRSpatialReferenceH, + ) -> Result<Self> { + let cloned = call_gdal_api!(api, OSRClone, c_srs); + if cloned.is_null() { + return Err(GdalError::NullPointer { + method_name: "OSRClone", + msg: "failed to clone spatial reference".to_string(), + }); + } + Ok(Self { api, c_srs: cloned }) + } + + /// Return the raw C handle. Review Comment: `c_srs()` exposes the raw `OGRSpatialReferenceH` without documenting the ownership/validity contract. To avoid accidental double-free or use-after-drop in downstream FFI, document that the handle is borrowed (owned by `SpatialRef`) and must not be released/destroyed by the caller, and is only valid for the lifetime of `&self`. ########## c/sedona-gdal/src/vsi.rs: ########## @@ -0,0 +1,186 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//! Ported (and contains copied code) from georust/gdal: +//! <https://github.com/georust/gdal/blob/v0.19.0/src/vsi.rs>. +//! Original code is licensed under MIT. +//! +//! GDAL Virtual File System (VSI) wrappers. + +use std::ffi::CString; + +use crate::errors::{GdalError, Result}; +use crate::gdal_api::{call_gdal_api, GdalApi}; + +/// Creates a new VSI in-memory file from a given buffer. +/// +/// The data is copied into GDAL-allocated memory (via `VSIMalloc`) so that +/// GDAL can safely free it with `VSIFree` when ownership is taken. +pub fn create_mem_file(api: &'static GdalApi, file_name: &str, data: &[u8]) -> Result<()> { + let c_file_name = CString::new(file_name)?; + let len = data.len(); + + // Allocate via GDAL's allocator so GDAL can safely free it. + let gdal_buf = unsafe { call_gdal_api!(api, VSIMalloc, len) } as *mut u8; + if gdal_buf.is_null() { + return Err(GdalError::NullPointer { + method_name: "VSIMalloc", + msg: format!("failed to allocate {len} bytes"), + }); Review Comment: `create_mem_file` calls `VSIMalloc(len)` even when `len == 0`. `malloc(0)` is allowed to return NULL, which would currently be treated as an allocation failure and prevent creating a valid empty `/vsimem/` file. Handle the zero-length case explicitly (e.g., avoid allocating/copying and pass a NULL pointer with length 0, or allocate a 1-byte buffer while still passing length 0). ########## c/sedona-gdal/src/vector/geometry.rs: ########## @@ -0,0 +1,149 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//! Ported (and contains copied code) from georust/gdal: +//! <https://github.com/georust/gdal/blob/v0.19.0/src/vector/geometry.rs>. +//! Original code is licensed under MIT. + +use std::ffi::CString; +use std::ptr; + +use crate::errors::{GdalError, Result}; +use crate::gdal_api::{call_gdal_api, GdalApi}; +use crate::gdal_dyn_bindgen::*; + +pub type Envelope = OGREnvelope; + +/// An OGR geometry. +pub struct Geometry { + api: &'static GdalApi, + c_geom: OGRGeometryH, +} + +unsafe impl Send for Geometry {} + +impl Drop for Geometry { + fn drop(&mut self) { + if !self.c_geom.is_null() { + unsafe { call_gdal_api!(self.api, OGR_G_DestroyGeometry, self.c_geom) }; + } + } +} + +impl Geometry { + /// Create a geometry from WKB bytes. + pub fn from_wkb(api: &'static GdalApi, wkb: &[u8]) -> Result<Self> { + let wkb_len: i32 = wkb.len().try_into()?; + let mut c_geom: OGRGeometryH = ptr::null_mut(); + let rv = unsafe { + call_gdal_api!( + api, + OGR_G_CreateFromWkb, + wkb.as_ptr() as *const std::ffi::c_void, + ptr::null_mut(), // hSRS + &mut c_geom, + wkb_len + ) + }; + if rv != OGRERR_NONE { + return Err(GdalError::OgrError { + err: rv, + method_name: "OGR_G_CreateFromWkb", + }); + } + if c_geom.is_null() { + return Err(GdalError::NullPointer { + method_name: "OGR_G_CreateFromWkb", + msg: "returned null geometry".to_string(), + }); + } + Ok(Self { api, c_geom }) + } + + /// Create a geometry from WKT string. + pub fn from_wkt(api: &'static GdalApi, wkt: &str) -> Result<Self> { + let c_wkt = CString::new(wkt)?; + let mut wkt_ptr = c_wkt.as_ptr() as *mut std::os::raw::c_char; + let mut c_geom: OGRGeometryH = ptr::null_mut(); + let rv = unsafe { + call_gdal_api!( + api, + OGR_G_CreateFromWkt, + &mut wkt_ptr, + ptr::null_mut(), // hSRS + &mut c_geom + ) + }; + if rv != OGRERR_NONE { + return Err(GdalError::OgrError { + err: rv, + method_name: "OGR_G_CreateFromWkt", + }); + } + if c_geom.is_null() { + return Err(GdalError::NullPointer { + method_name: "OGR_G_CreateFromWkt", + msg: "returned null geometry".to_string(), + }); + } + Ok(Self { api, c_geom }) + } + + /// Return the raw C geometry handle. Review Comment: `c_geometry()` exposes an owned raw `OGRGeometryH` without documenting the ownership/validity contract. Please document that the handle is borrowed from `Geometry`, must not be destroyed by the caller, and is only valid until `Geometry` is dropped (to prevent accidental double-free / UAF in downstream FFI). ########## c/sedona-gdal/src/vsi.rs: ########## @@ -0,0 +1,186 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//! Ported (and contains copied code) from georust/gdal: +//! <https://github.com/georust/gdal/blob/v0.19.0/src/vsi.rs>. +//! Original code is licensed under MIT. +//! +//! GDAL Virtual File System (VSI) wrappers. + +use std::ffi::CString; + +use crate::errors::{GdalError, Result}; +use crate::gdal_api::{call_gdal_api, GdalApi}; + +/// Creates a new VSI in-memory file from a given buffer. +/// +/// The data is copied into GDAL-allocated memory (via `VSIMalloc`) so that +/// GDAL can safely free it with `VSIFree` when ownership is taken. +pub fn create_mem_file(api: &'static GdalApi, file_name: &str, data: &[u8]) -> Result<()> { + let c_file_name = CString::new(file_name)?; + let len = data.len(); + + // Allocate via GDAL's allocator so GDAL can safely free it. + let gdal_buf = unsafe { call_gdal_api!(api, VSIMalloc, len) } as *mut u8; + if gdal_buf.is_null() { + return Err(GdalError::NullPointer { + method_name: "VSIMalloc", + msg: format!("failed to allocate {len} bytes"), + }); + } + + // Copy data into GDAL-allocated buffer + unsafe { + std::ptr::copy_nonoverlapping(data.as_ptr(), gdal_buf, len); + } + + let handle = unsafe { + call_gdal_api!( + api, + VSIFileFromMemBuffer, + c_file_name.as_ptr(), + gdal_buf, + len as i64, + 1 // bTakeOwnership = true — GDAL will VSIFree gdal_buf Review Comment: `len as i64` can truncate/wrap for large inputs (usize -> i64), which would pass an incorrect length to `VSIFileFromMemBuffer`. Prefer a checked conversion (e.g., `i64::try_from(len)?`) now that the crate supports `TryFromIntError` in `GdalError`. ########## c/sedona-gdal/src/spatial_ref.rs: ########## @@ -0,0 +1,156 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//! Ported (and contains copied code) from georust/gdal: +//! <https://github.com/georust/gdal/blob/v0.19.0/src/spatial_ref/srs.rs>. +//! Original code is licensed under MIT. + +use std::ffi::{CStr, CString}; +use std::ptr; + +use crate::errors::{GdalError, Result}; +use crate::gdal_api::{call_gdal_api, GdalApi}; +use crate::gdal_dyn_bindgen::OGRERR_NONE; +use crate::gdal_dyn_bindgen::*; + +/// An OGR spatial reference system. +pub struct SpatialRef { + api: &'static GdalApi, + c_srs: OGRSpatialReferenceH, +} + +unsafe impl Send for SpatialRef {} + +impl Drop for SpatialRef { + fn drop(&mut self) { + if !self.c_srs.is_null() { + unsafe { call_gdal_api!(self.api, OSRRelease, self.c_srs) }; + } + } +} + +impl SpatialRef { + /// Create a new SpatialRef from a WKT string. + pub fn from_wkt(api: &'static GdalApi, wkt: &str) -> Result<Self> { + let c_wkt = CString::new(wkt)?; + let c_srs = unsafe { call_gdal_api!(api, OSRNewSpatialReference, c_wkt.as_ptr()) }; + if c_srs.is_null() { + return Err(GdalError::NullPointer { + method_name: "OSRNewSpatialReference", + msg: "failed to create spatial reference from WKT".to_string(), + }); + } + Ok(Self { api, c_srs }) + } + + /// Create a SpatialRef by cloning a borrowed C handle via `OSRClone`. + /// + /// # Safety + /// + /// The caller must ensure `c_srs` is a valid `OGRSpatialReferenceH`. + pub unsafe fn from_c_srs_clone( + api: &'static GdalApi, + c_srs: OGRSpatialReferenceH, + ) -> Result<Self> { + let cloned = call_gdal_api!(api, OSRClone, c_srs); + if cloned.is_null() { + return Err(GdalError::NullPointer { + method_name: "OSRClone", + msg: "failed to clone spatial reference".to_string(), + }); + } + Ok(Self { api, c_srs: cloned }) + } + + /// Return the raw C handle. + pub fn c_srs(&self) -> OGRSpatialReferenceH { + self.c_srs + } + + /// Export to PROJJSON string. + pub fn to_projjson(&self) -> Result<String> { + unsafe { + let mut ptr: *mut std::os::raw::c_char = ptr::null_mut(); + let rv = call_gdal_api!( + self.api, + OSRExportToPROJJSON, + self.c_srs, + &mut ptr, + ptr::null() + ); + if rv != OGRERR_NONE || ptr.is_null() { + return Err(GdalError::NullPointer { + method_name: "OSRExportToPROJJSON", + msg: "returned null".to_string(), + }); + } Review Comment: `SpatialRef::to_projjson` returns `GdalError::NullPointer` when `OSRExportToPROJJSON` returns a non-success `OGRErr`. This loses the actual OGR error code and makes debugging harder. Consider returning `GdalError::OgrError { err: rv, method_name: ... }` when `rv != OGRERR_NONE`, and only using `NullPointer` when `rv == OGRERR_NONE` but the output pointer is NULL (also ensure any non-NULL output is freed on error). ########## c/sedona-gdal/src/vector/geometry.rs: ########## @@ -0,0 +1,149 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//! Ported (and contains copied code) from georust/gdal: +//! <https://github.com/georust/gdal/blob/v0.19.0/src/vector/geometry.rs>. +//! Original code is licensed under MIT. + +use std::ffi::CString; +use std::ptr; + +use crate::errors::{GdalError, Result}; +use crate::gdal_api::{call_gdal_api, GdalApi}; +use crate::gdal_dyn_bindgen::*; + +pub type Envelope = OGREnvelope; + +/// An OGR geometry. +pub struct Geometry { + api: &'static GdalApi, + c_geom: OGRGeometryH, +} + +unsafe impl Send for Geometry {} + +impl Drop for Geometry { + fn drop(&mut self) { + if !self.c_geom.is_null() { + unsafe { call_gdal_api!(self.api, OGR_G_DestroyGeometry, self.c_geom) }; + } + } +} + +impl Geometry { + /// Create a geometry from WKB bytes. + pub fn from_wkb(api: &'static GdalApi, wkb: &[u8]) -> Result<Self> { + let wkb_len: i32 = wkb.len().try_into()?; + let mut c_geom: OGRGeometryH = ptr::null_mut(); + let rv = unsafe { + call_gdal_api!( + api, + OGR_G_CreateFromWkb, + wkb.as_ptr() as *const std::ffi::c_void, + ptr::null_mut(), // hSRS + &mut c_geom, + wkb_len + ) + }; + if rv != OGRERR_NONE { + return Err(GdalError::OgrError { + err: rv, + method_name: "OGR_G_CreateFromWkb", + }); + } + if c_geom.is_null() { + return Err(GdalError::NullPointer { + method_name: "OGR_G_CreateFromWkb", + msg: "returned null geometry".to_string(), + }); + } + Ok(Self { api, c_geom }) + } Review Comment: This new geometry wrapper introduces parsing/export logic (`from_wkb`, `from_wkt`, `wkb`, `envelope`) but has no unit tests. Since other new primitives in this PR include tests, please add coverage for at least a round-trip (WKT -> Geometry -> WKB and/or WKB -> Geometry -> WKB), envelope correctness, and failure cases for invalid inputs. ########## c/sedona-gdal/src/vsi.rs: ########## @@ -0,0 +1,186 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//! Ported (and contains copied code) from georust/gdal: +//! <https://github.com/georust/gdal/blob/v0.19.0/src/vsi.rs>. +//! Original code is licensed under MIT. +//! +//! GDAL Virtual File System (VSI) wrappers. + +use std::ffi::CString; + +use crate::errors::{GdalError, Result}; +use crate::gdal_api::{call_gdal_api, GdalApi}; + +/// Creates a new VSI in-memory file from a given buffer. +/// +/// The data is copied into GDAL-allocated memory (via `VSIMalloc`) so that +/// GDAL can safely free it with `VSIFree` when ownership is taken. +pub fn create_mem_file(api: &'static GdalApi, file_name: &str, data: &[u8]) -> Result<()> { + let c_file_name = CString::new(file_name)?; + let len = data.len(); + + // Allocate via GDAL's allocator so GDAL can safely free it. + let gdal_buf = unsafe { call_gdal_api!(api, VSIMalloc, len) } as *mut u8; + if gdal_buf.is_null() { + return Err(GdalError::NullPointer { + method_name: "VSIMalloc", + msg: format!("failed to allocate {len} bytes"), + }); + } + + // Copy data into GDAL-allocated buffer + unsafe { + std::ptr::copy_nonoverlapping(data.as_ptr(), gdal_buf, len); + } + + let handle = unsafe { + call_gdal_api!( + api, + VSIFileFromMemBuffer, + c_file_name.as_ptr(), + gdal_buf, + len as i64, + 1 // bTakeOwnership = true — GDAL will VSIFree gdal_buf + ) + }; + + if handle.is_null() { + // GDAL did not take ownership, so we must free. + unsafe { call_gdal_api!(api, VSIFree, gdal_buf as *mut std::ffi::c_void) }; + return Err(GdalError::NullPointer { + method_name: "VSIFileFromMemBuffer", + msg: String::new(), + }); + } + + unsafe { + call_gdal_api!(api, VSIFCloseL, handle); + } + + Ok(()) +} + +/// Unlink (delete) a VSI in-memory file. +pub fn unlink_mem_file(api: &'static GdalApi, file_name: &str) -> Result<()> { + let c_file_name = CString::new(file_name)?; + + let rv = unsafe { call_gdal_api!(api, VSIUnlink, c_file_name.as_ptr()) }; + + if rv != 0 { + return Err(GdalError::UnlinkMemFile { + file_name: file_name.to_string(), + }); + } + + Ok(()) +} + +/// Copies the bytes of the VSI in-memory file, taking ownership and freeing the GDAL memory. +pub fn get_vsi_mem_file_bytes_owned(api: &'static GdalApi, file_name: &str) -> Result<Vec<u8>> { + let c_file_name = CString::new(file_name)?; + + let owned_bytes = unsafe { + let mut length: i64 = 0; + let bytes = call_gdal_api!( + api, + VSIGetMemFileBuffer, + c_file_name.as_ptr(), + &mut length, + 1 // bUnlinkAndSeize = true + ); + + if bytes.is_null() { + return Err(GdalError::NullPointer { + method_name: "VSIGetMemFileBuffer", + msg: String::new(), + }); + } + + if length < 0 { + call_gdal_api!(api, VSIFree, bytes.cast::<std::ffi::c_void>()); + return Err(GdalError::BadArgument(format!( + "VSIGetMemFileBuffer returned negative length: {length}" + ))); + } + + let slice = std::slice::from_raw_parts(bytes, length as usize); + let vec = slice.to_vec(); Review Comment: `length as usize` can silently wrap on 32-bit targets or for very large memfiles, leading to incorrect slicing and potential memory safety issues. Prefer a checked conversion (e.g., `usize::try_from(length)?`) before calling `from_raw_parts`, and return a conversion error if it doesn’t fit. ########## c/sedona-gdal/src/vector/geometry.rs: ########## @@ -0,0 +1,149 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//! Ported (and contains copied code) from georust/gdal: +//! <https://github.com/georust/gdal/blob/v0.19.0/src/vector/geometry.rs>. +//! Original code is licensed under MIT. + +use std::ffi::CString; +use std::ptr; + +use crate::errors::{GdalError, Result}; +use crate::gdal_api::{call_gdal_api, GdalApi}; +use crate::gdal_dyn_bindgen::*; + +pub type Envelope = OGREnvelope; + +/// An OGR geometry. +pub struct Geometry { + api: &'static GdalApi, + c_geom: OGRGeometryH, +} + Review Comment: The `unsafe impl Send for Geometry {}` should be accompanied by a safety justification (what invariants make it sound to move `OGRGeometryH` between threads, and why `Sync` is/ isn’t provided). Without a documented rationale, it’s hard to audit the correctness of this unsafety later. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
