Skip to content

Commit a7b0333

Browse files
committed
Fix memory leak of DynValue
1 parent d32d959 commit a7b0333

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
@@ -723,11 +723,11 @@ impl DatasetCreate {
723723
FillValue::Default | FillValue::UserDefined => {
724724
let dtype = Datatype::from_descriptor(tp)?;
725725
let mut buf: Vec<u8> = Vec::with_capacity(tp.size());
726+
h5try!(H5Pget_fill_value(self.id(), dtype.id(), buf.as_mut_ptr().cast()));
726727
unsafe {
727728
buf.set_len(tp.size());
728729
}
729-
h5try!(H5Pget_fill_value(self.id(), dtype.id(), buf.as_mut_ptr().cast()));
730-
Ok(Some(unsafe { OwnedDynValue::from_raw(tp.clone(), buf) }))
730+
Ok(Some(unsafe { OwnedDynValue::from_raw(tp.clone(), buf.into_boxed_slice()) }))
731731
}
732732
_ => Ok(None),
733733
}
@@ -741,11 +741,13 @@ impl DatasetCreate {
741741
pub fn get_fill_value_as<T: H5Type>(&self) -> Result<Option<T>> {
742742
let dtype = Datatype::from_type::<T>()?;
743743
Ok(self.get_fill_value(&dtype.to_descriptor()?)?.map(|value| unsafe {
744-
let mut out: T = mem::zeroed();
744+
let mut out: mem::MaybeUninit<T> = mem::MaybeUninit::uninit();
745745
let buf = value.get_buf();
746-
ptr::copy_nonoverlapping(buf.as_ptr(), (&mut out as *mut T).cast(), buf.len());
747-
mem::forget(value);
748-
out
746+
ptr::copy_nonoverlapping(buf.as_ptr(), out.as_mut_ptr().cast(), buf.len());
747+
// Drop the Box<[u8]>, but not subfields
748+
value.drop_nonrecursive();
749+
// We now have exclusive access to all subfields
750+
out.assume_init()
749751
}))
750752
}
751753

0 commit comments

Comments
 (0)