From 999e870d6417ed1a56d490a48b35472aa804d56e Mon Sep 17 00:00:00 2001 From: Ryan Lowe Date: Tue, 8 Aug 2023 23:42:59 -0400 Subject: [PATCH 1/6] Mark some functions as unsafe --- hdf5-sys/src/h5.rs | 2 +- hdf5-sys/src/h5a.rs | 4 +-- hdf5-sys/src/h5d.rs | 8 ++--- hdf5-sys/src/h5es.rs | 4 +-- hdf5-sys/src/h5fd.rs | 51 +++++++++++++++++--------------- hdf5-sys/src/h5i.rs | 16 +++++----- hdf5-sys/src/h5l.rs | 4 +-- hdf5-sys/src/h5mm.rs | 4 +-- hdf5-sys/src/h5o.rs | 2 +- hdf5-sys/src/h5p.rs | 18 ++++++----- hdf5-sys/src/h5vl.rs | 39 +++++++++++++----------- hdf5-sys/src/h5z.rs | 2 +- hdf5/src/error.rs | 2 +- hdf5/src/hl/attribute.rs | 5 ++-- hdf5/src/hl/chunks.rs | 2 +- hdf5/src/hl/filters/blosc.rs | 6 ++-- hdf5/src/hl/filters/lzf.rs | 2 +- hdf5/src/hl/group.rs | 2 +- hdf5/src/hl/plist.rs | 7 +++-- hdf5/src/hl/plist/file_access.rs | 14 +++++++-- hdf5/src/util.rs | 32 +++++++++++++++----- 21 files changed, 133 insertions(+), 93 deletions(-) diff --git a/hdf5-sys/src/h5.rs b/hdf5-sys/src/h5.rs index b06c31248..72ddd670e 100644 --- a/hdf5-sys/src/h5.rs +++ b/hdf5-sys/src/h5.rs @@ -117,7 +117,7 @@ extern "C" { } #[cfg(feature = "1.14.0")] -type H5_atclose_func_t = Option; +type H5_atclose_func_t = Option; #[cfg(feature = "1.14.0")] extern "C" { diff --git a/hdf5-sys/src/h5a.rs b/hdf5-sys/src/h5a.rs index 6a8b09b4c..d1c7f8f92 100644 --- a/hdf5-sys/src/h5a.rs +++ b/hdf5-sys/src/h5a.rs @@ -26,7 +26,7 @@ impl Default for H5A_info_t { #[deprecated(note = "deprecated in HDF5 1.8.0, use H5A_operator2_t")] pub type H5A_operator1_t = Option< - extern "C" fn( + unsafe extern "C" fn( location_id: hid_t, attr_name: *const c_char, operator_data: *mut c_void, @@ -34,7 +34,7 @@ pub type H5A_operator1_t = Option< >; pub type H5A_operator2_t = Option< - extern "C" fn( + unsafe extern "C" fn( location_id: hid_t, attr_name: *const c_char, ainfo: *const H5A_info_t, diff --git a/hdf5-sys/src/h5d.rs b/hdf5-sys/src/h5d.rs index 375467225..2a83e4149 100644 --- a/hdf5-sys/src/h5d.rs +++ b/hdf5-sys/src/h5d.rs @@ -125,7 +125,7 @@ pub enum H5D_mpio_no_collective_cause_t { } pub type H5D_operator_t = Option< - extern "C" fn( + unsafe extern "C" fn( elem: *mut c_void, type_id: hid_t, ndim: c_uint, @@ -136,7 +136,7 @@ pub type H5D_operator_t = Option< #[cfg(feature = "1.8.11")] pub type H5D_scatter_func_t = Option< - extern "C" fn( + unsafe extern "C" fn( src_buf: *mut *const c_void, src_buf_bytes_used: *mut size_t, op_data: *mut c_void, @@ -144,7 +144,7 @@ pub type H5D_scatter_func_t = Option< >; #[cfg(feature = "1.8.11")] pub type H5D_gather_func_t = Option< - extern "C" fn( + unsafe extern "C" fn( dst_buf: *const c_void, dst_buf_bytes_used: size_t, op_data: *mut c_void, @@ -289,7 +289,7 @@ extern "C" { #[cfg(feature = "1.14.0")] pub type H5D_chunk_iter_op_t = Option< - extern "C" fn( + unsafe extern "C" fn( offset: *const hsize_t, filter_mask: c_uint, addr: haddr_t, diff --git a/hdf5-sys/src/h5es.rs b/hdf5-sys/src/h5es.rs index 8ea81ad11..66a41e662 100644 --- a/hdf5-sys/src/h5es.rs +++ b/hdf5-sys/src/h5es.rs @@ -45,7 +45,7 @@ pub enum H5ES_status_t { } pub type H5ES_event_complete_func_t = Option< - extern "C" fn( + unsafe extern "C" fn( op_info: *mut H5ES_op_info_t, status: H5ES_status_t, err_stack: hid_t, @@ -54,7 +54,7 @@ pub type H5ES_event_complete_func_t = Option< >; pub type H5ES_event_insert_func_t = - Option c_int>; + Option c_int>; extern "C" { pub fn H5ESinsert_request(es_id: hid_t, connector_id: hid_t, request: *mut c_void) -> herr_t; diff --git a/hdf5-sys/src/h5fd.rs b/hdf5-sys/src/h5fd.rs index 59f9918ee..157d247ae 100644 --- a/hdf5-sys/src/h5fd.rs +++ b/hdf5-sys/src/h5fd.rs @@ -132,31 +132,33 @@ pub struct H5FD_class_t { pub fc_degree: H5F_close_degree_t, #[cfg(feature = "1.14.0")] pub terminate: Option herr_t>, - pub sb_size: Option hsize_t>, - pub sb_encode: - Option herr_t>, - pub sb_decode: - Option herr_t>, + pub sb_size: Option hsize_t>, + pub sb_encode: Option< + unsafe extern "C" fn(file: *mut H5FD_t, name: *mut c_char, p: *mut c_uchar) -> herr_t, + >, + pub sb_decode: Option< + unsafe extern "C" fn(f: *mut H5FD_t, name: *const c_char, p: *const c_uchar) -> herr_t, + >, pub fapl_size: size_t, - pub fapl_get: Option *mut c_void>, - pub fapl_copy: Option *mut c_void>, - pub fapl_free: Option herr_t>, + pub fapl_get: Option *mut c_void>, + pub fapl_copy: Option *mut c_void>, + pub fapl_free: Option herr_t>, pub dxpl_size: size_t, - pub dxpl_copy: Option *mut c_void>, - pub dxpl_free: Option herr_t>, + pub dxpl_copy: Option *mut c_void>, + pub dxpl_free: Option herr_t>, pub open: Option< - extern "C" fn( + unsafe extern "C" fn( name: *const c_char, flags: c_uint, fapl: hid_t, maxaddr: haddr_t, ) -> *mut H5FD_t, >, - pub close: Option herr_t>, - pub cmp: Option c_int>, - pub query: Option herr_t>, + pub close: Option herr_t>, + pub cmp: Option c_int>, + pub query: Option herr_t>, pub get_type_map: - Option herr_t>, + Option herr_t>, pub alloc: Option< extern "C" fn( file: *mut H5FD_t, @@ -174,10 +176,10 @@ pub struct H5FD_class_t { size: hsize_t, ) -> herr_t, >, - pub get_eoa: Option haddr_t>, + pub get_eoa: Option haddr_t>, pub set_eoa: - Option herr_t>, - pub get_eof: Option haddr_t>, + Option herr_t>, + pub get_eof: Option haddr_t>, pub get_handle: Option< extern "C" fn(file: *mut H5FD_t, fapl: hid_t, file_handle: *mut *mut c_void) -> herr_t, >, @@ -201,9 +203,10 @@ pub struct H5FD_class_t { buffer: *const c_void, ) -> herr_t, >, - pub flush: Option herr_t>, + pub flush: + Option herr_t>, pub truncate: - Option herr_t>, + Option herr_t>, pub lock: Option< extern "C" fn( file: *mut H5FD_t, @@ -213,9 +216,9 @@ pub struct H5FD_class_t { ) -> herr_t, >, pub unlock: - Option herr_t>, + Option herr_t>, #[cfg(feature = "1.14.0")] - pub del: Option herr_t>, + pub del: Option herr_t>, #[cfg(feature = "1.14.0")] pub ctl: Option< extern "C" fn( @@ -316,8 +319,8 @@ pub struct H5FD_file_image_callbacks_t { udata: *mut c_void, ) -> herr_t, >, - pub udata_copy: Option *mut c_void>, - pub udata_free: Option herr_t>, + pub udata_copy: Option *mut c_void>, + pub udata_free: Option herr_t>, pub udata: *mut c_void, } diff --git a/hdf5-sys/src/h5i.rs b/hdf5-sys/src/h5i.rs index d48899b9e..9fb1578a8 100644 --- a/hdf5-sys/src/h5i.rs +++ b/hdf5-sys/src/h5i.rs @@ -43,14 +43,14 @@ pub type hid_t = c_int; pub const H5I_INVALID_HID: hid_t = -1; #[cfg(not(feature = "1.14.0"))] -pub type H5I_free_t = Option herr_t>; +pub type H5I_free_t = Option herr_t>; #[cfg(feature = "1.14.0")] -pub type H5I_free_t = Option herr_t>; +pub type H5I_free_t = Option herr_t>; pub type H5I_search_func_t = - Option c_int>; + Option c_int>; #[cfg(feature = "1.12.0")] -pub type H5I_iterate_func_t = Option herr_t>; +pub type H5I_iterate_func_t = Option herr_t>; extern "C" { pub fn H5Iregister(type_: H5I_type_t, object: *const c_void) -> hid_t; @@ -79,11 +79,13 @@ extern "C" { } #[cfg(feature = "1.14.0")] -pub type H5I_future_realize_func_t = - Option herr_t>; +pub type H5I_future_realize_func_t = Option< + unsafe extern "C" fn(future_object: *mut c_void, actual_object_id: *mut hid_t) -> herr_t, +>; #[cfg(feature = "1.14.0")] -pub type H5I_future_discard_func_t = Option herr_t>; +pub type H5I_future_discard_func_t = + Option herr_t>; #[cfg(feature = "1.14.0")] extern "C" { diff --git a/hdf5-sys/src/h5l.rs b/hdf5-sys/src/h5l.rs index 96a9829df..ccca7afb5 100644 --- a/hdf5-sys/src/h5l.rs +++ b/hdf5-sys/src/h5l.rs @@ -169,7 +169,7 @@ impl Default for H5L_class_t { } pub type H5L_iterate1_t = Option< - extern "C" fn( + unsafe extern "C" fn( group: hid_t, name: *const c_char, info: *const H5L_info1_t, @@ -178,7 +178,7 @@ pub type H5L_iterate1_t = Option< >; #[cfg(feature = "1.12.0")] pub type H5L_iterate2_t = Option< - extern "C" fn( + unsafe extern "C" fn( group: hid_t, name: *const c_char, info: *const H5L_info2_t, diff --git a/hdf5-sys/src/h5mm.rs b/hdf5-sys/src/h5mm.rs index 8a0e861a8..0af33b273 100644 --- a/hdf5-sys/src/h5mm.rs +++ b/hdf5-sys/src/h5mm.rs @@ -2,5 +2,5 @@ use crate::internal_prelude::*; pub type H5MM_allocate_t = - Option *mut c_void>; -pub type H5MM_free_t = Option; + Option *mut c_void>; +pub type H5MM_free_t = Option; diff --git a/hdf5-sys/src/h5o.rs b/hdf5-sys/src/h5o.rs index 6137bf423..603fd9f53 100644 --- a/hdf5-sys/src/h5o.rs +++ b/hdf5-sys/src/h5o.rs @@ -204,7 +204,7 @@ pub enum H5O_mcdt_search_ret_t { #[cfg(feature = "1.8.9")] pub type H5O_mcdt_search_cb_t = - Option H5O_mcdt_search_ret_t>; + Option H5O_mcdt_search_ret_t>; #[cfg(not(feature = "1.10.3"))] extern "C" { diff --git a/hdf5-sys/src/h5p.rs b/hdf5-sys/src/h5p.rs index 572a8b643..1fd106ef1 100644 --- a/hdf5-sys/src/h5p.rs +++ b/hdf5-sys/src/h5p.rs @@ -26,13 +26,14 @@ pub const H5P_CRT_ORDER_INDEXED: c_uint = 0x0002; pub const H5P_DEFAULT: hid_t = 0; pub type H5P_cls_create_func_t = - Option herr_t>; -pub type H5P_cls_copy_func_t = - Option herr_t>; + Option herr_t>; +pub type H5P_cls_copy_func_t = Option< + unsafe extern "C" fn(new_prop_id: hid_t, old_prop_id: hid_t, copy_data: *mut c_void) -> herr_t, +>; pub type H5P_cls_close_func_t = - Option herr_t>; + Option herr_t>; pub type H5P_prp_cb1_t = - Option herr_t>; + Option herr_t>; pub type H5P_prp_cb2_t = Option< extern "C" fn(prop_id: hid_t, name: *const c_char, size: size_t, value: *mut c_void) -> herr_t, >; @@ -41,11 +42,12 @@ pub type H5P_prp_set_func_t = H5P_prp_cb2_t; pub type H5P_prp_get_func_t = H5P_prp_cb2_t; pub type H5P_prp_delete_func_t = H5P_prp_cb2_t; pub type H5P_prp_copy_func_t = H5P_prp_cb1_t; -pub type H5P_prp_compare_func_t = - Option c_int>; +pub type H5P_prp_compare_func_t = Option< + unsafe extern "C" fn(value1: *const c_void, value2: *const c_void, size: size_t) -> c_int, +>; pub type H5P_prp_close_func_t = H5P_prp_cb1_t; pub type H5P_iterate_t = - Option herr_t>; + Option herr_t>; pub use self::globals::*; diff --git a/hdf5-sys/src/h5vl.rs b/hdf5-sys/src/h5vl.rs index c27879793..56446df7d 100644 --- a/hdf5-sys/src/h5vl.rs +++ b/hdf5-sys/src/h5vl.rs @@ -131,7 +131,7 @@ mod v1_14_0 { #[derive(Debug, Copy, Clone)] pub struct H5VL_info_class_t { pub size: size_t, - pub copy: Option *mut c_void>, + pub copy: Option *mut c_void>, pub cmp: Option< extern "C" fn( cmp_value: *mut c_int, @@ -139,17 +139,19 @@ mod v1_14_0 { info2: *const c_void, ) -> herr_t, >, - pub free: Option herr_t>, - pub to_str: Option herr_t>, - pub from_str: Option herr_t>, + pub free: Option herr_t>, + pub to_str: + Option herr_t>, + pub from_str: + Option herr_t>, } #[repr(C)] #[derive(Debug, Copy, Clone)] pub struct H5VL_wrap_class_t { - pub get_object: Option *mut c_void>, + pub get_object: Option *mut c_void>, pub get_wrap_ctx: - Option herr_t>, + Option herr_t>, pub wrap_object: Option< extern "C" fn( obj: *mut c_void, @@ -157,8 +159,8 @@ mod v1_14_0 { wrap_ctx: *mut c_void, ) -> *mut c_void, >, - pub unwrap_object: Option *mut c_void>, - pub free_wrap_ctx: Option herr_t>, + pub unwrap_object: Option *mut c_void>, + pub free_wrap_ctx: Option herr_t>, } #[repr(C)] @@ -762,8 +764,9 @@ mod v1_14_0 { req: *mut *mut c_void, ) -> herr_t, >, - pub close: - Option herr_t>, + pub close: Option< + unsafe extern "C" fn(dt: *mut c_void, dxpl_id: hid_t, req: *mut *mut c_void) -> herr_t, + >, } #[repr(C)] @@ -1602,7 +1605,7 @@ mod v1_14_0 { ) -> herr_t, >, pub get_cap_flags: - Option herr_t>, + Option herr_t>, pub opt_query: Option< extern "C" fn( obj: *mut c_void, @@ -1668,7 +1671,7 @@ mod v1_14_0 { ); pub type H5VL_request_notify_t = - Option herr_t>; + Option herr_t>; #[repr(C)] #[derive(Debug, Copy, Clone)] @@ -1683,14 +1686,16 @@ mod v1_14_0 { pub notify: Option< extern "C" fn(req: *mut c_void, cb: H5VL_request_notify_t, ctx: *mut c_void) -> herr_t, >, - pub cancel: - Option herr_t>, + pub cancel: Option< + unsafe extern "C" fn(req: *mut c_void, status: *mut H5VL_request_status_t) -> herr_t, + >, pub specific: Option< extern "C" fn(req: *mut c_void, args: *mut H5VL_request_specific_args_t) -> herr_t, >, - pub optional: - Option herr_t>, - pub free: Option herr_t>, + pub optional: Option< + unsafe extern "C" fn(req: *mut c_void, args: *mut H5VL_optional_args_t) -> herr_t, + >, + pub free: Option herr_t>, } #[repr(C)] diff --git a/hdf5-sys/src/h5z.rs b/hdf5-sys/src/h5z.rs index 69ffbcd84..740569140 100644 --- a/hdf5-sys/src/h5z.rs +++ b/hdf5-sys/src/h5z.rs @@ -114,7 +114,7 @@ pub type H5Z_can_apply_func_t = pub type H5Z_set_local_func_t = Option herr_t>; pub type H5Z_func_t = Option< - extern "C" fn( + unsafe extern "C" fn( flags: c_uint, cd_nelmts: size_t, cd_values: *const c_uint, diff --git a/hdf5/src/error.rs b/hdf5/src/error.rs index 553a6dbe4..339a63398 100644 --- a/hdf5/src/error.rs +++ b/hdf5/src/error.rs @@ -68,7 +68,7 @@ impl ErrorStack { stack: ExpandedErrorStack, err: Option, } - extern "C" fn callback( + unsafe extern "C" fn callback( _: c_uint, err_desc: *const H5E_error2_t, data: *mut c_void, ) -> herr_t { panic::catch_unwind(|| unsafe { diff --git a/hdf5/src/hl/attribute.rs b/hdf5/src/hl/attribute.rs index 8943e0aa8..a7a457e97 100644 --- a/hdf5/src/hl/attribute.rs +++ b/hdf5/src/hl/attribute.rs @@ -48,13 +48,14 @@ impl Deref for Attribute { impl Attribute { /// Returns names of all the members in the group, non-recursively. pub fn attr_names(obj: &Location) -> Result> { - extern "C" fn attributes_callback( + unsafe extern "C" fn attributes_callback( _id: hid_t, attr_name: *const c_char, _info: *const H5A_info_t, op_data: *mut c_void, ) -> herr_t { std::panic::catch_unwind(|| { let other_data: &mut Vec = unsafe { &mut *(op_data.cast::>()) }; - other_data.push(string_from_cstr(attr_name)); + // SAFETY: caller guarantees attr_name points to valid UTF-8 C string + other_data.push(unsafe { string_from_cstr(attr_name) }); 0 // Continue iteration }) .unwrap_or(-1) diff --git a/hdf5/src/hl/chunks.rs b/hdf5/src/hl/chunks.rs index a84ac68ae..ac20be48b 100644 --- a/hdf5/src/hl/chunks.rs +++ b/hdf5/src/hl/chunks.rs @@ -105,7 +105,7 @@ mod v1_14_0 { pub callback: F, } - extern "C" fn chunks_callback( + unsafe extern "C" fn chunks_callback( offset: *const hsize_t, filter_mask: c_uint, addr: haddr_t, size: hsize_t, op_data: *mut c_void, ) -> herr_t diff --git a/hdf5/src/hl/filters/blosc.rs b/hdf5/src/hl/filters/blosc.rs index f1ea55e5d..c3ec828d5 100644 --- a/hdf5/src/hl/filters/blosc.rs +++ b/hdf5/src/hl/filters/blosc.rs @@ -160,13 +160,13 @@ fn parse_blosc_cdata(cd_nelmts: size_t, cd_values: *const c_uint) -> Option= 7 { let r = unsafe { blosc_compcode_to_compname(cdata[6] as _, addr_of_mut!(cfg.compname)) }; if r == -1 { - let complist = string_from_cstr(unsafe { blosc_list_compressors() }); + let complist = unsafe { string_from_cstr(blosc_list_compressors()) }; let errmsg = format!( concat!( "This Blosc library does not have support for the '{}' compressor, ", "but only for: {}" ), - string_from_cstr(cfg.compname), + unsafe { string_from_cstr(cfg.compname) }, complist ); h5err!(errmsg, H5E_PLIST, H5E_CALLBACK); @@ -176,7 +176,7 @@ fn parse_blosc_cdata(cd_nelmts: size_t, cd_values: *const c_uint) -> Option size_t { diff --git a/hdf5/src/hl/filters/lzf.rs b/hdf5/src/hl/filters/lzf.rs index 14751c715..6bd6e8b0c 100644 --- a/hdf5/src/hl/filters/lzf.rs +++ b/hdf5/src/hl/filters/lzf.rs @@ -92,7 +92,7 @@ extern "C" fn set_local_lzf(dcpl_id: hid_t, type_id: hid_t, _space_id: hid_t) -> } } -extern "C" fn filter_lzf( +unsafe extern "C" fn filter_lzf( flags: c_uint, cd_nelmts: size_t, cd_values: *const c_uint, nbytes: size_t, buf_size: *mut size_t, buf: *mut *mut c_void, ) -> size_t { diff --git a/hdf5/src/hl/group.rs b/hdf5/src/hl/group.rs index 9bcbe28c6..7626c9b45 100644 --- a/hdf5/src/hl/group.rs +++ b/hdf5/src/hl/group.rs @@ -312,7 +312,7 @@ impl Group { // Maps a closure to a C callback // // This function will be called multiple times, but never concurrently - extern "C" fn callback( + unsafe extern "C" fn callback( id: hid_t, name: *const c_char, info: *const H5L_info_t, op_data: *mut c_void, ) -> herr_t where diff --git a/hdf5/src/hl/plist.rs b/hdf5/src/hl/plist.rs index abd4da83b..f80ea91c2 100644 --- a/hdf5/src/hl/plist.rs +++ b/hdf5/src/hl/plist.rs @@ -171,10 +171,11 @@ impl PropertyList { /// Iterates over properties in the property list, returning their names. pub fn properties(&self) -> Vec { - extern "C" fn callback(_: hid_t, name: *const c_char, data: *mut c_void) -> herr_t { + unsafe extern "C" fn callback(_: hid_t, name: *const c_char, data: *mut c_void) -> herr_t { panic::catch_unwind(|| { let data = unsafe { &mut *(data.cast::>()) }; - let name = string_from_cstr(name); + // SAFETY: caller guarantees name is a valid CStr and UTF-8 + let name = unsafe { string_from_cstr(name) }; if !name.is_empty() { data.push(name); } @@ -244,7 +245,7 @@ pub fn set_vlen_manager_libc(plist: hid_t) -> Result<()> { extern "C" fn alloc(size: size_t, _info: *mut c_void) -> *mut c_void { panic::catch_unwind(|| unsafe { libc::malloc(size) }).unwrap_or(ptr::null_mut()) } - extern "C" fn free(ptr: *mut c_void, _info: *mut libc::c_void) { + unsafe extern "C" fn free(ptr: *mut c_void, _info: *mut libc::c_void) { let _p = panic::catch_unwind(|| unsafe { libc::free(ptr); }); diff --git a/hdf5/src/hl/plist/file_access.rs b/hdf5/src/hl/plist/file_access.rs index 6d0e050bc..a05f29fc5 100644 --- a/hdf5/src/hl/plist/file_access.rs +++ b/hdf5/src/hl/plist/file_access.rs @@ -1566,12 +1566,19 @@ impl FileAccess { ensure!(j < N, "member map index out of bounds: {} (expected 0-{})", j, N - 1); if mapping[j] == 0 { mapping[j] = 0xff - (files.len() as u8); - files.push(MultiFile::new(&string_from_cstr(name), addr as _)); + files.push(MultiFile::new( + // SAFETY: name produced by HDF5 is nul-terminated and valid UTF-8 + unsafe { &string_from_cstr(name) }, + addr as _, + )); } *layout.get_mut(i - 1) = 0xff - mapping[j]; } for &memb_name in &memb_name { - crate::util::h5_free_memory(memb_name as *mut _); + // SAFETY: the array contains pointers to strings allocated by the previous H5P call + unsafe { + crate::util::h5_free_memory(memb_name as *mut _); + } } let relax = relax > 0; let drv = MultiDriver { files, layout, relax }; @@ -1783,7 +1790,8 @@ impl FileAccess { )); Ok(CacheLogOptions { is_enabled: is_enabled > 0, - location: string_from_cstr(buf.as_ptr()), + // SAFETY: buf points to a valid UTF-8 CStr created by previous H5P call + location: unsafe { string_from_cstr(buf.as_ptr()) }, start_on_access: start_on_access > 0, }) } diff --git a/hdf5/src/util.rs b/hdf5/src/util.rs index c919b87eb..99498abe7 100644 --- a/hdf5/src/util.rs +++ b/hdf5/src/util.rs @@ -7,7 +7,10 @@ use std::str; use crate::internal_prelude::*; /// Convert a zero-terminated string (`const char *`) into a `String`. -pub fn string_from_cstr(string: *const c_char) -> String { +/// # Safety +/// The memory pointed to by `string` must be valid for constructing a `CStr` +/// containing valid UTF-8. +pub unsafe fn string_from_cstr(string: *const c_char) -> String { unsafe { String::from_utf8_unchecked(CStr::from_ptr(string).to_bytes().to_vec()) } } @@ -19,10 +22,12 @@ pub fn to_cstring>(string: S) -> Result { } /// Convert a fixed-length (possibly zero-terminated) char buffer to a string. +/// # Panics +/// Panics if the bytes are not valid UTF-8. pub fn string_from_fixed_bytes(bytes: &[c_char], len: usize) -> String { let len = bytes.iter().position(|&c| c == 0).unwrap_or(len); let bytes = &bytes[..len]; - let s = unsafe { str::from_utf8_unchecked(&*(bytes as *const [c_char] as *const [u8])) }; + let s = unsafe { str::from_utf8(&*(bytes as *const [c_char] as *const [u8])).unwrap() }; s.to_owned() } @@ -41,21 +46,31 @@ pub fn string_to_fixed_bytes(s: &str, buf: &mut [c_char]) { } } +/// # Safety +/// `mem` must point to memory allocated by HDF5. #[cfg(feature = "1.8.13")] -pub fn h5_free_memory(mem: *mut c_void) { +pub unsafe fn h5_free_memory(mem: *mut c_void) { use hdf5_sys::h5::H5free_memory; unsafe { H5free_memory(mem) }; } +/// # Safety +/// `mem` must point to memory allocated by HDF5. #[cfg(not(feature = "1.8.13"))] -pub fn h5_free_memory(mem: *mut c_void) { +pub unsafe fn h5_free_memory(mem: *mut c_void) { // this may fail in debug builds of HDF5 use libc::free; unsafe { free(mem) }; } +/// # Safety +/// `func` must expect a pointer to a buffer and its size. +/// If the pointer is null, `func` must return the length of the message. +/// Otherwise, `func` must try to write a string into the buffer that is valid for constructing +/// a `CStr` and contain valid UTF-8. It must succeed and return the length of the string if the +/// buffer is large enough, or return a negative value if it is too small. #[doc(hidden)] -pub fn get_h5_str(func: F) -> Result +pub unsafe fn get_h5_str(func: F) -> Result where F: Fn(*mut c_char, size_t) -> T, T: TryInto, @@ -67,6 +82,7 @@ where } else { let mut buf = vec![0; len as usize]; func(buf.as_mut_ptr(), len as _); + // SAFETY: buf contains a valid UTF-8 C string Ok(string_from_cstr(buf.as_ptr())) } } @@ -85,10 +101,12 @@ mod tests { pub fn test_string_cstr() { let s1 = "foo".to_owned(); let c_s1 = to_cstring(s1.clone()).unwrap(); - assert_eq!(s1, string_from_cstr(c_s1.as_ptr())); + // SAFETY: c_s1 is a valid C string created from a String + assert_eq!(s1, unsafe { string_from_cstr(c_s1.as_ptr()) }); let s2 = "bar"; let c_s2 = to_cstring(s2).unwrap(); - assert_eq!(s2, string_from_cstr(c_s2.as_ptr())); + // SAFETY: c_s2 is a valid C string created from a String + assert_eq!(s2, unsafe { string_from_cstr(c_s2.as_ptr()) }); } #[test] From 4759891839d959c27e5bfede516c80165066461e Mon Sep 17 00:00:00 2001 From: Magnus Ulimoen Date: Tue, 26 Sep 2023 21:06:58 +0200 Subject: [PATCH 2/6] Only conditionally call H5Pset_evict_on_close --- hdf5/src/hl/plist/file_access.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/hdf5/src/hl/plist/file_access.rs b/hdf5/src/hl/plist/file_access.rs index 6d0e050bc..578600619 100644 --- a/hdf5/src/hl/plist/file_access.rs +++ b/hdf5/src/hl/plist/file_access.rs @@ -1440,8 +1440,13 @@ impl FileAccessBuilder { v.min_raw_perc as _, )); } - if let Some(v) = self.evict_on_close { - h5try!(H5Pset_evict_on_close(id, hbool_t::from(v))); + if let Some(evict) = self.evict_on_close { + // Issue #259: H5Pset_evict_on_close is not allowed to be called + // even if the argument is `false` on e.g. parallel/mpio setups + let has_evict_on_close = h5get!(H5Pget_evict_on_close(id): hbool_t).map(|x| x > 0); + if evict != has_evict_on_close.unwrap_or(false) { + h5try!(H5Pset_evict_on_close(id, hbool_t::from(evict))); + } } if let Some(v) = self.mdc_image_config { let v = v.into(); From 6a41c3ce449ea843351bab243088a51edffa11dc Mon Sep 17 00:00:00 2001 From: Magnus Ulimoen Date: Mon, 2 Oct 2023 19:23:27 +0200 Subject: [PATCH 3/6] Add to changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e760225a8..3f08790f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,7 @@ - Applying filters without chunking will now produce an explicit error. - Fixed a bug where chunking could not be enabled for zero-sized extents. - Fixed library finding on Windows with MSYS2-distributed MinGW HDF5. +- Fixed a bug which made parallel builds unusable. ## 0.8.1 From ba12781f67bd0ef9d501b5c53522a1d902a9cbab Mon Sep 17 00:00:00 2001 From: Magnus Ulimoen Date: Mon, 2 Oct 2023 19:24:49 +0200 Subject: [PATCH 4/6] Bump MSRV to 1.70 --- .github/workflows/ci.yml | 2 +- CHANGELOG.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9c3247496..8fd2022f6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -273,7 +273,7 @@ jobs: with: {submodules: true} - name: Install Rust uses: dtolnay/rust-toolchain@stable - with: {toolchain: 1.64} + with: {toolchain: "1.70"} - name: Build and test all crates run: cargo test --workspace -vv --features=hdf5-sys/static,hdf5-sys/zlib --exclude=hdf5-derive diff --git a/CHANGELOG.md b/CHANGELOG.md index 3f08790f1..d7deca09f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +16,7 @@ ### Changed - The `H5Type` derive macro now uses `proc-macro-error` to emit error messages. -- MSRV is now `1.64.0` and Rust edition has now been bumped to 2021. +- MSRV is now `1.70.0` and Rust edition has now been bumped to 2021. - Types in ChunkInfo has been changed to match HDF5. - Dependencies now uses the `dep:` syntax and are only enabled through features. - Some features are made weak and will not enable e.g. static build when asking for a From 396a2cf30b3aba53abc82a23489275b6399b1030 Mon Sep 17 00:00:00 2001 From: Sophie Hotz Date: Wed, 8 Nov 2023 08:06:24 +0100 Subject: [PATCH 5/6] Fix definition of `H5L_info2_t` Replace incorrect `H5L_info1_t__u` with `H5L_info2_t__u`. Since `H5L_info1_t__u` does not have the same size as `H5L_info2_t__u`, this bug resulted in 8 bytes of stack memory outside the allocated region being overwritten by the hdf5 C-library for example when using `H5Lget_info2`. Since `Debug` can no longer be derived, add a manual implementation for the struct `H5L_info2_t` in order to correctly match the variant of `H5L_info2_t__u` to the link type. Additionally, add methods to access the union fields of `H5L_info2_t__u`. --- hdf5-sys/src/h5l.rs | 38 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 36 insertions(+), 2 deletions(-) diff --git a/hdf5-sys/src/h5l.rs b/hdf5-sys/src/h5l.rs index 96a9829df..7593fa7ea 100644 --- a/hdf5-sys/src/h5l.rs +++ b/hdf5-sys/src/h5l.rs @@ -1,4 +1,5 @@ //! Creating and manipulating links within an HDF5 group +use std::fmt; use std::mem; pub use self::H5L_type_t::*; @@ -64,14 +65,14 @@ impl H5L_info1_t__u { } #[repr(C)] -#[derive(Debug, Copy, Clone)] +#[derive(Copy, Clone)] #[cfg(feature = "1.12.0")] pub struct H5L_info2_t { pub type_: H5L_type_t, pub corder_valid: hbool_t, pub corder: int64_t, pub cset: H5T_cset_t, - pub u: H5L_info1_t__u, + pub u: H5L_info2_t__u, } #[cfg(feature = "1.12.0")] @@ -81,6 +82,29 @@ impl Default for H5L_info2_t { } } +#[cfg(feature = "1.12.0")] +impl fmt::Debug for H5L_info2_t { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + let mut debug_struct = f.debug_struct("H5L_info2_t"); + debug_struct + .field("type_", &self.type_) + .field("corder_valid", &self.corder_valid) + .field("corder", &self.corder) + .field("cset", &self.cset); + + match self.type_ { + H5L_TYPE_HARD => { + debug_struct.field("token", unsafe { &self.u.token }); + } + H5L_TYPE_SOFT | H5L_TYPE_EXTERNAL | H5L_TYPE_MAX => { + debug_struct.field("val_size", unsafe { &self.u.val_size }); + } + H5L_TYPE_ERROR => {} + } + debug_struct.finish() + } +} + #[repr(C)] #[derive(Copy, Clone)] #[cfg(feature = "1.12.0")] @@ -96,6 +120,16 @@ impl Default for H5L_info2_t__u { } } +#[cfg(feature = "1.12.0")] +impl H5L_info2_t__u { + pub unsafe fn token(&mut self) -> *mut H5O_token_t { + &mut self.token as *mut H5O_token_t + } + pub unsafe fn val_size(&mut self) -> *mut size_t { + &mut self.val_size as *mut size_t + } +} + pub type H5L_create_func_t = Option< extern "C" fn( link_name: *const c_char, From cde72f383fdf257ed3dcdc5e80707ee71b247173 Mon Sep 17 00:00:00 2001 From: Sophie Hotz Date: Wed, 8 Nov 2023 13:52:44 +0100 Subject: [PATCH 6/6] fixup! Fix definition of `H5L_info2_t` --- hdf5-sys/src/h5l.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/hdf5-sys/src/h5l.rs b/hdf5-sys/src/h5l.rs index 7593fa7ea..a07da6be5 100644 --- a/hdf5-sys/src/h5l.rs +++ b/hdf5-sys/src/h5l.rs @@ -1,4 +1,5 @@ //! Creating and manipulating links within an HDF5 group +#[cfg(feature = "1.12.0")] use std::fmt; use std::mem;