Copilot commented on code in PR #4872:
URL: https://github.com/apache/arrow-adbc/pull/4872#discussion_r4215432348
##########
rust/ffi/src/driver_exporter.rs:
##########
@@ -1949,3 +1953,96 @@ unsafe extern "C" fn error_get_detail(
}
}
}
+
+#[cfg(test)]
+mod tests {
+ use super::*;
+ use crate::types::FFI_AdbcErrorV100;
+ use std::ptr::{null, null_mut};
+
+ #[test]
+ fn test_error_details_null() {
+ unsafe {
+ assert_eq!(error_get_detail_count(null()), 0);
+ for index in [-1, 0, 1] {
+ let detail = error_get_detail(null(), index);
+ assert!(detail.key.is_null());
+ assert!(detail.value.is_null());
+ assert_eq!(detail.value_length, 0);
+ }
+ }
+ }
+
+ #[test]
+ fn test_error_details_legacy() {
+ for vendor_code in [0, 42, -1] {
+ let error = Box::new(FFI_AdbcErrorV100 {
+ message: null_mut(),
+ vendor_code,
+ sqlstate: [0; 5],
+ release: None,
+ });
+ let error =
std::ptr::from_ref(error.as_ref()).cast::<FFI_AdbcError>();
+ unsafe {
+ assert_eq!(error_get_detail_count(error), 0);
+ for index in [-1, 0, 1] {
+ let detail = error_get_detail(error, index);
+ assert!(detail.key.is_null());
+ assert!(detail.value.is_null());
+ assert_eq!(detail.value_length, 0);
+ }
+ }
+ }
+ }
+
+ #[test]
+ fn test_error_details_uninitialized() {
+ let error = FFI_AdbcError::default();
+ unsafe {
+ assert_eq!(error_get_detail_count(&error), 0);
+ for index in [-1, 0, 1] {
+ let detail = error_get_detail(&error, index);
+ assert!(detail.key.is_null());
+ assert!(detail.value.is_null());
+ assert_eq!(detail.value_length, 0);
+ }
+ }
+ }
+
+ #[test]
+ fn test_error_details_extended() -> std::result::Result<(),
std::ffi::NulError> {
+ let value = b"value\0binary";
+ let mut error = FFI_AdbcError::try_from(Error {
+ message: "test error".into(),
+ status: Status::Unknown,
+ vendor_code: 42,
+ sqlstate: [0; 5],
+ details: Some(vec![("key".into(), value.to_vec())]),
+ })?;
+
+ unsafe {
+ assert_eq!(error_get_detail_count(&error), 0);
+ let detail = error_get_detail(&error, 0);
+ assert!(detail.key.is_null());
+ assert!(detail.value.is_null());
+ assert_eq!(detail.value_length, 0);
+
+ error.vendor_code = ADBC_ERROR_VENDOR_CODE_PRIVATE_DATA;
+ assert_eq!(error_get_detail_count(&error), 1);
+ let detail = error_get_detail(&error, 0);
+ assert_eq!(CStr::from_ptr(detail.key).to_bytes(), b"key");
+ assert_eq!(detail.value_length, value.len());
+ assert_eq!(
+ std::slice::from_raw_parts(detail.value, detail.value_length),
+ value
+ );
+ for index in [-1, 1, c_int::MAX] {
+ let detail = error_get_detail(&error, index);
+ assert!(detail.key.is_null());
+ assert!(detail.value.is_null());
+ assert_eq!(detail.value_length, 0);
+ }
+ }
Review Comment:
This test constructs an FFI error whose message and detail storage are
transferred with `CString::into_raw`/`Box::into_raw`, but it never invokes the
embedded release callback. `FFI_AdbcError` has no `Drop` implementation, so the
test leaks both allocations; the adjacent exporter tests explicitly call
`release` for this reason. Release the error after the final assertions.
--
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]
