Skip to content

Commit 9ad0925

Browse files
committed
Fix memory leak of DynValue
1 parent 02e3fa7 commit 9ad0925

File tree

2 files changed

+21
-9
lines changed

2 files changed

+21
-9
lines changed

hdf5-types/src/dyn_value.rs

+13-3
Original file line numberDiff line numberDiff line change
@@ -702,7 +702,7 @@ impl Display for DynValue<'_> {
702702

703703
pub struct OwnedDynValue {
704704
tp: TypeDescriptor,
705-
buf: Vec<u8>,
705+
buf: Box<[u8]>,
706706
}
707707

708708
impl OwnedDynValue {
@@ -711,7 +711,7 @@ impl OwnedDynValue {
711711
let len = mem::size_of_val(&value);
712712
let buf = unsafe { std::slice::from_raw_parts(ptr, len) };
713713
mem::forget(value);
714-
Self { tp: T::type_descriptor(), buf: buf.to_owned() }
714+
Self { tp: T::type_descriptor(), buf: buf.to_owned().into_boxed_slice() }
715715
}
716716

717717
pub fn get(&self) -> DynValue {
@@ -728,9 +728,19 @@ impl OwnedDynValue {
728728
}
729729

730730
#[doc(hidden)]
731-
pub unsafe fn from_raw(tp: TypeDescriptor, buf: Vec<u8>) -> Self {
731+
pub unsafe fn from_raw(tp: TypeDescriptor, buf: Box<[u8]>) -> Self {
732732
Self { tp, buf }
733733
}
734+
735+
#[doc(hidden)]
736+
/// Use this if the values should still be used after the buffer has been
737+
/// copied to the concrete type
738+
pub fn drop_nonrecursive(mut self) {
739+
// We can't take ownership of the contents of self,
740+
// but we can replace the items with a zero-sized object
741+
self.tp = <[u8; 0] as H5Type>::type_descriptor();
742+
self.buf = Box::new([]);
743+
}
734744
}
735745

736746
impl<T: H5Type> From<T> for OwnedDynValue {

src/hl/plist/dataset_create.rs

+8-6
Original file line numberDiff line numberDiff line change
@@ -722,11 +722,11 @@ impl DatasetCreate {
722722
FillValue::Default | FillValue::UserDefined => {
723723
let dtype = Datatype::from_descriptor(tp)?;
724724
let mut buf: Vec<u8> = Vec::with_capacity(tp.size());
725+
h5try!(H5Pget_fill_value(self.id(), dtype.id(), buf.as_mut_ptr().cast()));
725726
unsafe {
726727
buf.set_len(tp.size());
727728
}
728-
h5try!(H5Pget_fill_value(self.id(), dtype.id(), buf.as_mut_ptr().cast()));
729-
Ok(Some(unsafe { OwnedDynValue::from_raw(tp.clone(), buf) }))
729+
Ok(Some(unsafe { OwnedDynValue::from_raw(tp.clone(), buf.into_boxed_slice()) }))
730730
}
731731
FillValue::Undefined => Ok(None),
732732
}
@@ -740,11 +740,13 @@ impl DatasetCreate {
740740
pub fn get_fill_value_as<T: H5Type>(&self) -> Result<Option<T>> {
741741
let dtype = Datatype::from_type::<T>()?;
742742
Ok(self.get_fill_value(&dtype.to_descriptor()?)?.map(|value| unsafe {
743-
let mut out: T = mem::zeroed();
743+
let mut out: mem::MaybeUninit<T> = mem::MaybeUninit::uninit();
744744
let buf = value.get_buf();
745-
ptr::copy_nonoverlapping(buf.as_ptr(), (&mut out as *mut T).cast(), buf.len());
746-
mem::forget(value);
747-
out
745+
ptr::copy_nonoverlapping(buf.as_ptr(), out.as_mut_ptr().cast(), buf.len());
746+
// Drop the Box<[u8]>, but not subfields
747+
value.drop_nonrecursive();
748+
// We now have exclusive access to all subfields
749+
out.assume_init()
748750
}))
749751
}
750752

0 commit comments

Comments
 (0)