Skip to content

Commit 1228604

Browse files
joseph-isaacsjoseph-isaacs
authored
Remove unsafe ptr deref and use as_ref (#2959)
1 parent 7ff6487 commit 1228604

File tree

5 files changed

+52
-39
lines changed

5 files changed

+52
-39
lines changed

‎vortex-ffi/src/array.rs‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ pub struct FFIArray {
2222
/// Get the length of the array.
2323
#[unsafe(no_mangle)]
2424
pub unsafe extern "C" fn FFIArray_len(ffi_array: *const FFIArray) -> u64 {
25-
let array = &*ffi_array;
25+
let array = ffi_array.as_ref().vortex_expect("array null");
2626

2727
array.inner.len() as u64
2828
}
@@ -33,7 +33,7 @@ pub unsafe extern "C" fn FFIArray_len(ffi_array: *const FFIArray) -> u64 {
3333
/// for ensuring that it is never dereferenced after the array has been freed.
3434
#[unsafe(no_mangle)]
3535
pub unsafe extern "C" fn FFIArray_dtype(ffi_array: *const FFIArray) -> *const DType {
36-
let array = &*ffi_array;
36+
let array = ffi_array.as_ref().vortex_expect("array null");
3737

3838
array.inner.dtype()
3939
}
@@ -43,7 +43,7 @@ pub unsafe extern "C" fn FFIArray_get_field(
4343
ffi_array: *const FFIArray,
4444
index: u32,
4545
) -> *const FFIArray {
46-
let array = &*ffi_array;
46+
let array = ffi_array.as_ref().vortex_expect("array null");
4747

4848
let field_array = array
4949
.inner
@@ -75,15 +75,15 @@ pub unsafe extern "C" fn FFIArray_slice(
7575
start: u32,
7676
stop: u32,
7777
) -> *mut FFIArray {
78-
let array = &*array;
78+
let array = array.as_ref().vortex_expect("array null");
7979
let sliced = slice(array.inner.as_ref(), start as usize, stop as usize)
8080
.vortex_expect("FFIArray_slice: slice");
8181
Box::into_raw(Box::new(FFIArray { inner: sliced }))
8282
}
8383

8484
#[unsafe(no_mangle)]
8585
pub unsafe extern "C" fn FFIArray_is_null(array: *const FFIArray, index: u32) -> bool {
86-
let array = &*array;
86+
let array = array.as_ref().vortex_expect("array null");
8787
array
8888
.inner
8989
.is_invalid(index as usize)
@@ -92,7 +92,7 @@ pub unsafe extern "C" fn FFIArray_is_null(array: *const FFIArray, index: u32) ->
9292

9393
#[unsafe(no_mangle)]
9494
pub unsafe extern "C" fn FFIArray_null_count(array: *const FFIArray) -> u32 {
95-
let array = &*array;
95+
let array = array.as_ref().vortex_expect("array null");
9696
array
9797
.inner
9898
.as_ref()
@@ -107,7 +107,7 @@ macro_rules! ffiarray_get_ptype {
107107
paste::paste! {
108108
#[unsafe(no_mangle)]
109109
pub unsafe extern "C" fn [<FFIArray_get_ $ptype>](array: *const FFIArray, index: u32) -> $ptype {
110-
let array = &*array;
110+
let array = array.as_ref().vortex_expect("array null");
111111
let value = scalar_at(array.inner.as_ref(), index as usize).vortex_expect("scalar_at");
112112
value.as_primitive()
113113
.as_::<$ptype>()
@@ -117,7 +117,7 @@ macro_rules! ffiarray_get_ptype {
117117

118118
#[unsafe(no_mangle)]
119119
pub unsafe extern "C" fn [<FFIArray_get_storage_ $ptype>](array: *const FFIArray, index: u32) -> $ptype {
120-
let array = &*array;
120+
let array = array.as_ref().vortex_expect("array null");
121121
let value = scalar_at(array.inner.as_ref(), index as usize).vortex_expect("scalar_at");
122122
value.as_extension()
123123
.storage()
@@ -151,7 +151,7 @@ pub unsafe extern "C" fn FFIArray_get_utf8(
151151
dst: *mut c_void,
152152
len: *mut c_int,
153153
) {
154-
let array = &*array;
154+
let array = array.as_ref().vortex_expect("array null");
155155
let value = scalar_at(array.inner.as_ref(), index as usize).vortex_expect("scalar_at");
156156
let utf8_scalar = value.as_utf8();
157157
if let Some(buffer) = utf8_scalar.value() {
@@ -171,7 +171,7 @@ pub unsafe extern "C" fn FFIArray_get_binary(
171171
dst: *mut c_void,
172172
len: *mut c_int,
173173
) {
174-
let array = &*array;
174+
let array = array.as_ref().vortex_expect("array null");
175175
let value = scalar_at(array.inner.as_ref(), index as usize).vortex_expect("scalar_at");
176176
let utf8_scalar = value.as_binary();
177177
if let Some(bytes) = utf8_scalar.value() {

‎vortex-ffi/src/dtype.rs‎

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ pub unsafe extern "C" fn DType_free(dtype: *mut DType) {
9090
/// Get the dtype variant tag for an [`DType`].
9191
#[unsafe(no_mangle)]
9292
pub unsafe extern "C" fn DType_get(dtype: *const DType) -> u8 {
93-
match &*dtype {
93+
match dtype.as_ref().vortex_expect("null dtype") {
9494
DType::Null => DTYPE_NULL,
9595
DType::Bool(_) => DTYPE_BOOL,
9696
DType::Primitive(ptype, _) => match ptype {
@@ -116,14 +116,15 @@ pub unsafe extern "C" fn DType_get(dtype: *const DType) -> u8 {
116116

117117
#[unsafe(no_mangle)]
118118
pub unsafe extern "C" fn DType_nullable(dtype: *const DType) -> bool {
119-
let dtype = &*dtype;
119+
let dtype = unsafe { dtype.as_ref() }.vortex_expect("dtype null");
120120
dtype.is_nullable()
121121
}
122122

123123
/// For `DTYPE_STRUCT` variant DTypes, get the number of fields.
124124
#[unsafe(no_mangle)]
125125
pub unsafe extern "C" fn DType_field_count(dtype: *const DType) -> u32 {
126-
let DType::Struct(struct_dtype, _) = &*dtype else {
126+
let DType::Struct(struct_dtype, _) = unsafe { dtype.as_ref() }.vortex_expect("dtype null")
127+
else {
127128
panic!("FFIDType_field_count: not a struct dtype")
128129
};
129130

@@ -143,7 +144,8 @@ pub unsafe extern "C" fn DType_field_name(
143144
assert!(!dst.is_null(), "DType_field_name: null ptr dst");
144145
assert!(!len.is_null(), "DType_field_name: null ptr len");
145146

146-
let DType::Struct(struct_dtype, _) = &*dtype else {
147+
let dtype = unsafe { dtype.as_ref() }.vortex_expect("dtype null");
148+
let DType::Struct(struct_dtype, _) = dtype else {
147149
panic!("FFIDType_field_name: not a struct dtype")
148150
};
149151

@@ -162,7 +164,8 @@ pub unsafe extern "C" fn DType_field_name(
162164
/// by the caller.
163165
#[unsafe(no_mangle)]
164166
pub unsafe extern "C" fn DType_field_dtype(dtype: *const DType, index: u32) -> *mut DType {
165-
let DType::Struct(struct_dtype, _) = &*dtype else {
167+
let dtype = unsafe { dtype.as_ref() }.vortex_expect("dtype null");
168+
let DType::Struct(struct_dtype, _) = dtype else {
166169
panic!("DType_field_dtype: not a struct dtype")
167170
};
168171

@@ -184,7 +187,8 @@ pub unsafe extern "C" fn DType_field_dtype(dtype: *const DType, index: u32) -> *
184187
pub unsafe extern "C" fn DType_element_type(dtype: *const DType) -> *const DType {
185188
assert!(!dtype.is_null(), "DType_element_type: null ptr");
186189

187-
let DType::List(element_dtype, _) = &*dtype else {
190+
let dtype = unsafe { dtype.as_ref() }.vortex_expect("dtype null");
191+
let DType::List(element_dtype, _) = dtype else {
188192
panic!("DType_element_type: not a list dtype")
189193
};
190194

@@ -193,31 +197,39 @@ pub unsafe extern "C" fn DType_element_type(dtype: *const DType) -> *const DType
193197

194198
#[unsafe(no_mangle)]
195199
pub unsafe extern "C" fn DType_is_time(dtype: *const DType) -> bool {
196-
match &*dtype {
200+
let dtype = unsafe { dtype.as_ref() }.vortex_expect("dtype null");
201+
202+
match dtype {
197203
DType::Extension(ext_dtype) => ext_dtype.id() == &*TIME_ID,
198204
_ => false,
199205
}
200206
}
201207

202208
#[unsafe(no_mangle)]
203209
pub unsafe extern "C" fn DType_is_date(dtype: *const DType) -> bool {
204-
match &*dtype {
210+
let dtype = unsafe { dtype.as_ref() }.vortex_expect("dtype null");
211+
212+
match dtype {
205213
DType::Extension(ext_dtype) => ext_dtype.id() == &*DATE_ID,
206214
_ => false,
207215
}
208216
}
209217

210218
#[unsafe(no_mangle)]
211219
pub unsafe extern "C" fn DType_is_timestamp(dtype: *const DType) -> bool {
212-
match &*dtype {
220+
let dtype = unsafe { dtype.as_ref() }.vortex_expect("dtype null");
221+
222+
match dtype {
213223
DType::Extension(ext_dtype) => ext_dtype.id() == &*TIMESTAMP_ID,
214224
_ => false,
215225
}
216226
}
217227

218228
#[unsafe(no_mangle)]
219229
pub unsafe extern "C" fn DType_time_unit(dtype: *const DType) -> u8 {
220-
let DType::Extension(ext_dtype) = &*dtype else {
230+
let dtype = unsafe { dtype.as_ref() }.vortex_expect("dtype null");
231+
232+
let DType::Extension(ext_dtype) = dtype else {
221233
panic!("DType_time_unit: not a time dtype")
222234
};
223235

@@ -229,7 +241,9 @@ pub unsafe extern "C" fn DType_time_unit(dtype: *const DType) -> u8 {
229241

230242
#[unsafe(no_mangle)]
231243
pub unsafe extern "C" fn DType_time_zone(dtype: *const DType, dst: *mut c_void, len: *mut c_int) {
232-
let DType::Extension(ext_dtype) = &*dtype else {
244+
let dtype = unsafe { dtype.as_ref() }.vortex_expect("dtype null");
245+
246+
let DType::Extension(ext_dtype) = dtype else {
233247
panic!("DType_time_unit: not a time dtype")
234248
};
235249

‎vortex-ffi/src/duckdb/mod.rs‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ use crate::to_string;
2323

2424
#[unsafe(no_mangle)]
2525
pub unsafe extern "C" fn DType_to_duckdb_logical_type(dtype: *mut DType) -> duckdb_logical_type {
26-
let dtype = unsafe { &*dtype };
26+
let dtype = unsafe { dtype.as_ref().vortex_expect("null dtype") };
2727

2828
dtype
2929
.to_duckdb_type()
@@ -43,7 +43,10 @@ pub unsafe extern "C" fn FFIArray_to_duckdb_chunk(
4343
cache: *mut FFIConversionCache,
4444
) -> c_uint {
4545
let offset = offset as usize;
46-
let array = unsafe { &(*stream).inner };
46+
47+
let array = &unsafe { stream.as_ref() }
48+
.vortex_expect("null stream")
49+
.inner;
4750

4851
assert!(array.len() > offset, "offset out of bounds");
4952

@@ -103,7 +106,7 @@ pub unsafe extern "C" fn FFIArray_append_duckdb_chunk(
103106
array: *mut FFIArray,
104107
chunk: duckdb_data_chunk,
105108
) -> *mut FFIArray {
106-
let array = unsafe { &*array };
109+
let array = unsafe { array.as_ref().vortex_expect("null array") };
107110

108111
let struct_type = array
109112
.inner

‎vortex-ffi/src/file.rs‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -70,9 +70,7 @@ pub struct FileScanOptions {
7070
/// Open a file at the given path on the file system.
7171
#[unsafe(no_mangle)]
7272
pub unsafe extern "C" fn File_open(options: *const FileOpenOptions) -> *mut FFIFile {
73-
assert!(!options.is_null(), "File_open: null options");
74-
75-
let options = &*options;
73+
let options = unsafe { options.as_ref().vortex_expect("null options") };
7674

7775
assert!(!options.uri.is_null(), "File_open: null uri");
7876
let uri = CStr::from_ptr(options.uri).to_string_lossy();
@@ -104,12 +102,12 @@ pub unsafe extern "C" fn File_create_and_write_array(
104102
options: *const FileCreateOptions,
105103
ffi_array: *mut FFIArray,
106104
) {
107-
let options = &*options;
105+
let options = unsafe { options.as_ref().vortex_expect("null options") };
108106

109107
assert!(!options.path.is_null(), "File_open: null uri");
110108
let path = CStr::from_ptr(options.path).to_string_lossy();
111109

112-
let array = unsafe { &*ffi_array };
110+
let array = unsafe { ffi_array.as_ref().vortex_expect("null array") };
113111

114112
RUNTIME.block_on(async move {
115113
let file = tokio::fs::File::create(path.to_string())
@@ -149,10 +147,7 @@ pub unsafe extern "C" fn FileStatistics_free(stat: *mut FileStatistics) {
149147
/// dereferenced after the file has been freed.
150148
#[unsafe(no_mangle)]
151149
pub unsafe extern "C" fn File_dtype(file: *const FFIFile) -> *const DType {
152-
assert!(!file.is_null(), "File_dtype: file is null");
153-
154-
let file = &*file;
155-
file.inner.dtype()
150+
file.as_ref().vortex_expect("null file").inner.dtype()
156151
}
157152

158153
/// Build a new `FFIArrayStream` that return a series of `FFIArray`s scan over a `FFIFile`.

‎vortex-ffi/src/stream.rs‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,8 @@ pub struct FFIArrayStreamInner {
2121

2222
#[unsafe(no_mangle)]
2323
pub unsafe extern "C" fn FFIArrayStream_dtype(stream: *const FFIArrayStream) -> *const DType {
24-
let stream = &*stream;
25-
stream
24+
unsafe { stream.as_ref() }
25+
.vortex_expect("null stream")
2626
.inner
2727
.as_ref()
2828
.vortex_expect("FFIArrayStream_dtype: called after finish")
@@ -38,7 +38,7 @@ pub unsafe extern "C" fn FFIArrayStream_dtype(stream: *const FFIArrayStream) ->
3838
/// It is an error to call this function again after the stream is finished.
3939
#[unsafe(no_mangle)]
4040
pub unsafe extern "C" fn FFIArrayStream_next(stream: *mut FFIArrayStream) -> bool {
41-
let stream = &mut *stream;
41+
let stream = unsafe { stream.as_mut() }.vortex_expect("stream null");
4242
let inner = stream
4343
.inner
4444
.as_mut()
@@ -66,8 +66,9 @@ pub unsafe extern "C" fn FFIArrayStream_next(stream: *mut FFIArrayStream) -> boo
6666
/// Predicate function to check if the array stream is finished.
6767
#[unsafe(no_mangle)]
6868
pub unsafe extern "C" fn FFIArrayStream_finished(stream: *const FFIArrayStream) -> bool {
69-
let stream = &*stream;
70-
stream.inner.is_none()
69+
unsafe { stream.as_ref().vortex_expect("null stream") }
70+
.inner
71+
.is_none()
7172
}
7273

7374
/// Get the current array batch from the stream. Returns a unique pointer.
@@ -79,7 +80,7 @@ pub unsafe extern "C" fn FFIArrayStream_finished(stream: *const FFIArrayStream)
7980
/// This function is unsafe because it dereferences the `stream` pointer.
8081
#[unsafe(no_mangle)]
8182
pub unsafe extern "C" fn FFIArrayStream_current(stream: *mut FFIArrayStream) -> *mut FFIArray {
82-
let stream = &mut *stream;
83+
let stream = unsafe { stream.as_mut().vortex_expect("null stream") };
8384

8485
let current = stream
8586
.current

0 commit comments

Comments
 (0)