Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 68 additions & 3 deletions rust/ffi/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -560,17 +560,22 @@ unsafe extern "C" fn release_ffi_error(error: *mut FFI_AdbcError) {
match error.as_mut() {
None => (),
Some(error) => {
// SAFETY: `error.message` was necessarily obtained with `CString::into_raw`.
// Additionally, C should not modify the string's length.
drop(CString::from_raw(error.message));
if !error.message.is_null() {
// SAFETY: `error.message` was necessarily obtained with `CString::into_raw`.
// Additionally, C should not modify the string's length.
drop(CString::from_raw(error.message));
error.message = null_mut();
}

if !error.private_data.is_null() {
// SAFETY: `error.private_data` was necessarily obtained with `Box::into_raw`.
// Additionally, the boxed data is necessarily `ErrorPrivateData`.
// Finally, C should call the release function only once.
let private_data = Box::from_raw(error.private_data as *mut ErrorPrivateData);
drop(private_data);
error.private_data = null_mut();
}
error.release = None;
}
}
}
Expand Down Expand Up @@ -626,4 +631,64 @@ mod tests {
let partitions_actual: Partitions = partitions_ffi.into();
assert_eq!(partitions_expected, partitions_actual);
}

#[test]
fn test_release_ffi_error_nulls_fields() {
// Use details to ensure private_data is non-null before release.
let error = Error {
message: "test error".into(),
status: Status::Unknown,
vendor_code: 0,
sqlstate: [0; 5],
details: Some(vec![("key".to_string(), b"value".to_vec())]),
};
let mut ffi_error: FFI_AdbcError = error.try_into().unwrap();
assert!(!ffi_error.message.is_null());
assert!(!ffi_error.private_data.is_null());

unsafe { release_ffi_error(&mut ffi_error) };

assert!(ffi_error.message.is_null());
assert!(ffi_error.private_data.is_null());
assert!(ffi_error.release.is_none());
// Drop here is a no-op because release is None.
}

#[test]
fn test_release_ffi_error_idempotent() {
// Calling release twice must not double-free the message pointer.
let error = Error {
message: "test error".into(),
status: Status::Unknown,
vendor_code: 0,
sqlstate: [0; 5],
details: None,
};
let mut ffi_error: FFI_AdbcError = error.try_into().unwrap();

unsafe {
release_ffi_error(&mut ffi_error);
release_ffi_error(&mut ffi_error);
}
// Drop here is a no-op because release is None.
}

#[test]
fn test_release_ffi_error_null_message() {
// A null message must not cause UB or a panic during release.
let mut ffi_error = FFI_AdbcError {
message: null_mut(),
vendor_code: 0,
sqlstate: [0; 5],
release: Some(release_ffi_error),
private_data: null_mut(),
private_driver: null(),
};

unsafe { release_ffi_error(&mut ffi_error) };

assert!(ffi_error.message.is_null());
assert!(ffi_error.release.is_none());
// Drop here is a no-op because release is None.
}
}
Loading