diff --git a/crates/duckdb/src/core/logical_type.rs b/crates/duckdb/src/core/logical_type.rs index 8d718744..b1dadd02 100644 --- a/crates/duckdb/src/core/logical_type.rs +++ b/crates/duckdb/src/core/logical_type.rs @@ -321,7 +321,7 @@ impl LogicalTypeHandle { /// Logical type child name by idx /// - /// Panics if the logical type is not a struct or union + /// Panics if the logical type is not a struct or union, or if `idx` is out of range. pub fn child_name(&self, idx: usize) -> String { unsafe { let child_name_ptr = match self.id() { @@ -330,7 +330,10 @@ impl LogicalTypeHandle { LogicalTypeId::Unsupported => panic!("unsupported logical type {}", self.raw_id()), _ => panic!("not a struct or union"), }; - let c_str = CString::from_raw(child_name_ptr); + if child_name_ptr.is_null() { + panic!("child index {idx} out of range"); + } + let c_str = DuckDbString::from_ptr(child_name_ptr); let name = c_str.to_str().unwrap(); name.to_string() } @@ -354,13 +357,10 @@ impl LogicalTypeHandle { pub fn get_alias(&self) -> Option { unsafe { let alias_ptr = duckdb_logical_type_get_alias(self.ptr); - if alias_ptr.is_null() { - None - } else { - let c_str = CString::from_raw(alias_ptr); + DuckDbString::from_nullable_ptr(alias_ptr).map(|c_str| { let alias = c_str.to_str().unwrap(); - Some(alias.to_string()) - } + alias.to_string() + }) } } diff --git a/crates/duckdb/src/core/vector.rs b/crates/duckdb/src/core/vector.rs index 0718e45f..5a612c93 100644 --- a/crates/duckdb/src/core/vector.rs +++ b/crates/duckdb/src/core/vector.rs @@ -428,10 +428,15 @@ impl<'a> StructVector<'a> { } /// Get the name of the child by idx. + /// + /// Panics if `idx` is out of range. pub fn child_name(&self, idx: usize) -> DuckDbString { let logical_type = self.logical_type(); unsafe { let child_name_ptr = duckdb_struct_type_child_name(logical_type.ptr, idx as u64); + if child_name_ptr.is_null() { + panic!("child index {idx} out of range"); + } DuckDbString::from_ptr(child_name_ptr) } } diff --git a/crates/duckdb/src/inner_connection.rs b/crates/duckdb/src/inner_connection.rs index a60cd52c..42584515 100644 --- a/crates/duckdb/src/inner_connection.rs +++ b/crates/duckdb/src/inner_connection.rs @@ -1,5 +1,5 @@ use std::{ - ffi::{CStr, CString, c_void}, + ffi::{CStr, CString}, mem, os::raw::c_char, ptr, str, @@ -92,8 +92,7 @@ impl InnerConnection { let mut c_err = std::ptr::null_mut(); let r = ffi::duckdb_open_ext(c_path.as_ptr(), &mut db, config.duckdb_config(), &mut c_err); if r != ffi::DuckDBSuccess { - let msg = Some(CStr::from_ptr(c_err).to_string_lossy().to_string()); - ffi::duckdb_free(c_err as *mut c_void); + let msg = ffi::DuckDbString::from_nullable_ptr(c_err).map(|c_err| c_err.to_string_lossy().to_string()); return Err(Error::DuckDBFailure(ffi::Error::new(r), msg)); } Self::new_from_raw_db(db, true) diff --git a/crates/duckdb/src/profiling.rs b/crates/duckdb/src/profiling.rs index bd22e50c..9af8b9aa 100644 --- a/crates/duckdb/src/profiling.rs +++ b/crates/duckdb/src/profiling.rs @@ -59,6 +59,7 @@ pub struct ProfilingInfo { impl ProfilingInfo { /// # Safety /// `info` must be a valid (or NULL) pointer obtained from [`libduckdb_sys::duckdb_get_profiling_info`]. + /// Metric key/value string conversions are treated as fallible. NULL varchar results are skipped. fn from_raw(info: libduckdb_sys::duckdb_profiling_info) -> Option { if info.is_null() { return None; @@ -74,20 +75,9 @@ impl ProfilingInfo { let mut key = unsafe { libduckdb_sys::duckdb_get_map_key(map, i) }; let mut val = unsafe { libduckdb_sys::duckdb_get_map_value(map, i) }; - let key_mem = unsafe { libduckdb_sys::duckdb_get_varchar(key) }; - let val_mem = unsafe { libduckdb_sys::duckdb_get_varchar(val) }; - - let key_str = unsafe { std::ffi::CStr::from_ptr(key_mem) } - .to_string_lossy() - .to_string(); - let val_str = unsafe { std::ffi::CStr::from_ptr(val_mem) } - .to_string_lossy() - .to_string(); - - metrics.insert(key_str, val_str); - - unsafe { libduckdb_sys::duckdb_free(key_mem as *mut std::ffi::c_void) }; - unsafe { libduckdb_sys::duckdb_free(val_mem as *mut std::ffi::c_void) }; + if let (Some(key_str), Some(val_str)) = (Self::value_to_string(key), Self::value_to_string(val)) { + metrics.insert(key_str, val_str); + } unsafe { libduckdb_sys::duckdb_destroy_value(&mut key) }; unsafe { libduckdb_sys::duckdb_destroy_value(&mut val) }; @@ -107,6 +97,12 @@ impl ProfilingInfo { Some(ProfilingInfo { metrics, children }) } + + fn value_to_string(value: libduckdb_sys::duckdb_value) -> Option { + let ptr = unsafe { libduckdb_sys::duckdb_get_varchar(value) }; + unsafe { libduckdb_sys::DuckDbString::from_nullable_ptr(ptr) } + .map(|varchar| varchar.to_string_lossy().to_string()) + } } impl InnerConnection { diff --git a/crates/duckdb/src/raw_statement.rs b/crates/duckdb/src/raw_statement.rs index eaea4779..18b53d87 100644 --- a/crates/duckdb/src/raw_statement.rs +++ b/crates/duckdb/src/raw_statement.rs @@ -286,7 +286,10 @@ impl RawStatement { print!("row-value:"); for col_idx in 0..duckdb_column_count(&mut result) { let val = ffi::duckdb_value_varchar(&mut result, col_idx, row_idx); - print!("{} ", CStr::from_ptr(val).to_string_lossy()); + match ffi::DuckDbString::from_nullable_ptr(val) { + Some(val) => print!("{} ", val.to_string_lossy()), + None => print!("NULL "), + } } println!(); } @@ -377,16 +380,15 @@ impl RawStatement { unsafe { let name_ptr = ffi::duckdb_parameter_name(self.ptr, idx as u64); // Range check above ensures this shouldn't be null, but check defensively - if name_ptr.is_null() { - return Err(Error::DuckDBFailure( - ffi::Error::new(ffi::DuckDBError), - Some(format!("Could not retrieve parameter name for index {idx}")), - )); - } - - let name = CStr::from_ptr(name_ptr).to_string_lossy().to_string(); - - ffi::duckdb_free(name_ptr as *mut std::ffi::c_void); + let name = match ffi::DuckDbString::from_nullable_ptr(name_ptr) { + Some(name) => name.to_string_lossy().to_string(), + None => { + return Err(Error::DuckDBFailure( + ffi::Error::new(ffi::DuckDBError), + Some(format!("Could not retrieve parameter name for index {idx}")), + )); + } + }; Ok(name) } diff --git a/crates/duckdb/src/vtab/function.rs b/crates/duckdb/src/vtab/function.rs index a6e3f2ea..36176f8f 100644 --- a/crates/duckdb/src/vtab/function.rs +++ b/crates/duckdb/src/vtab/function.rs @@ -293,9 +293,12 @@ impl TableFunction { /// /// # Arguments /// * `name`: The name of the table function + /// + /// # Panics + /// Panics if `name` contains interior NUL bytes. pub fn set_name(&self, name: &str) -> &Self { + let string = CString::new(name).expect("table function name must not contain NUL bytes"); unsafe { - let string = CString::from_vec_unchecked(name.as_bytes().into()); duckdb_table_function_set_name(self.ptr, string.as_ptr()); } self diff --git a/crates/duckdb/src/vtab/value.rs b/crates/duckdb/src/vtab/value.rs index e126a8cd..62e5e2b7 100644 --- a/crates/duckdb/src/vtab/value.rs +++ b/crates/duckdb/src/vtab/value.rs @@ -1,11 +1,11 @@ use crate::core::LogicalTypeId; use crate::ffi::{ - duckdb_destroy_value, duckdb_free, duckdb_get_bool, duckdb_get_double, duckdb_get_float, duckdb_get_int8, + DuckDbString, duckdb_destroy_value, duckdb_get_bool, duckdb_get_double, duckdb_get_float, duckdb_get_int8, duckdb_get_int16, duckdb_get_int32, duckdb_get_int64, duckdb_get_list_child, duckdb_get_list_size, duckdb_get_type_id, duckdb_get_uint8, duckdb_get_uint16, duckdb_get_uint32, duckdb_get_uint64, duckdb_get_value_type, duckdb_get_varchar, duckdb_is_null_value, duckdb_value, }; -use std::{ffi::CStr, fmt, os::raw::c_void}; +use std::fmt; /// The Value object holds a single arbitrary value of any type that can be /// stored in the database. @@ -101,11 +101,10 @@ impl Value { impl fmt::Display for Value { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { unsafe { - let varchar = duckdb_get_varchar(self.ptr); - let c_str = CStr::from_ptr(varchar); - let res = write!(f, "{}", c_str.to_string_lossy()); - duckdb_free(varchar as *mut c_void); - res + match DuckDbString::from_nullable_ptr(duckdb_get_varchar(self.ptr)) { + Some(varchar) => write!(f, "{}", varchar.to_string_lossy()), + None => f.write_str("NULL"), + } } } } diff --git a/crates/libduckdb-sys/src/string.rs b/crates/libduckdb-sys/src/string.rs index b4b18732..36a67169 100644 --- a/crates/libduckdb-sys/src/string.rs +++ b/crates/libduckdb-sys/src/string.rs @@ -16,24 +16,47 @@ impl DuckDbString { /// /// # Safety /// - /// The caller must ensure that the pointer is valid and points to a null-terminated C string. - /// The memory must remain valid for the lifetime of the returned `DuckDbString`. + /// The caller must ensure that: + /// - `ptr` is non-null and points to a null-terminated C string. + /// - `ptr` was allocated by DuckDB and must be released with `duckdb_free`. + /// - Ownership of `ptr` transfers to the returned `DuckDbString`. The caller must not reuse or free it. pub unsafe fn from_ptr(ptr: *const c_char) -> Self { + assert!(!ptr.is_null(), "DuckDbString::from_ptr requires a non-null pointer"); let len = unsafe { CStr::from_ptr(ptr) }.to_bytes().len(); unsafe { Self::from_raw_parts(ptr, len) } } + /// Creates a `DuckDbString` from a nullable raw pointer to a C string. + /// + /// Returns `None` if `ptr` is null. + /// + /// # Safety + /// + /// If `ptr` is non-null, the caller must ensure that: + /// - `ptr` points to a null-terminated C string. + /// - `ptr` was allocated by DuckDB and must be released with `duckdb_free`. + /// - Ownership of `ptr` transfers to the returned `DuckDbString`. The caller must not reuse or free it. + pub unsafe fn from_nullable_ptr(ptr: *const c_char) -> Option { + if ptr.is_null() { + None + } else { + Some(unsafe { Self::from_ptr(ptr) }) + } + } + /// Creates a `DuckDbString` from raw parts. /// /// # Safety /// /// The caller must ensure that: - /// - `ptr` is a valid pointer to a null-terminated C string. + /// - `ptr` is non-null and points to a null-terminated C string. /// - `len` accurately represents the length of the string (excluding the null terminator). - /// - The memory referenced by `ptr` remains valid for the lifetime of the returned `DuckDbString`. + /// - `ptr` was allocated by DuckDB and must be released with `duckdb_free`. + /// - Ownership of `ptr` transfers to the returned `DuckDbString`. The caller must not reuse or free it. /// - The string data is not mutated for the lifetime of the returned `DuckDbString`. pub unsafe fn from_raw_parts(ptr: *const c_char, len: usize) -> Self { - let ptr = unsafe { core::ptr::NonNull::new_unchecked(ptr as *mut c_char) }; + let ptr = core::ptr::NonNull::new(ptr as *mut c_char) + .expect("DuckDbString::from_raw_parts requires a non-null pointer"); Self { ptr, len } } @@ -58,3 +81,13 @@ impl Drop for DuckDbString { unsafe { duckdb_free(ptr) }; } } + +#[cfg(test)] +mod tests { + use super::DuckDbString; + + #[test] + fn from_nullable_ptr_returns_none_for_null() { + assert!(unsafe { DuckDbString::from_nullable_ptr(std::ptr::null()) }.is_none()); + } +}