refactor: TskBox::new now returns Result (#553)

Author: molpopgen

src/sys/table_collection.rs

@@ -11,17 +11,18 @@ use super::bindings::tsk_table_collection_free;
 use super::bindings::tsk_table_collection_init;
 use super::bindings::tsk_table_collection_t;
 use super::tskbox::TskBox;
+use super::Error;
 
 pub struct TableCollection(TskBox<tsk_table_collection_t>);
 
 impl TableCollection {
-    pub fn new(sequence_length: f64) -> Self {
+    pub fn new(sequence_length: f64) -> Result<Self, Error> {
         let mut tsk = TskBox::new(
             |tc: *mut tsk_table_collection_t| unsafe { tsk_table_collection_init(tc, 0) },
             tsk_table_collection_free,
-        );
+        )?;
         tsk.as_mut().sequence_length = sequence_length;
-        Self(tsk)
+        Ok(Self(tsk))
     }
 
     // # Safety

src/sys/tables.rs

@@ -49,7 +49,8 @@ macro_rules! basic_llowningtable_impl {
                 let table = super::tskbox::TskBox::new(
                     |x: *mut super::bindings::$tsktable| unsafe { super::bindings::$init(x, 0) },
                     super::bindings::$free,
-                );
+                )
+                .unwrap();
                 Self(table)
             }
 

src/sys/tree.rs

@@ -21,7 +21,7 @@ impl<'treeseq> LLTree<'treeseq> {
                 super::bindings::tsk_tree_init(x, treeseq.as_ref(), flags.bits())
             },
             super::bindings::tsk_tree_free,
-        );
+        )?;
         // Gotta ask Jerome about this one--why isn't this handled in tsk_tree_init??
         if !flags.contains(TreeFlags::NO_SAMPLE_COUNTS) {
             // SAFETY: nobody is null here.

src/sys/tskbox.rs

@@ -1,5 +1,7 @@
 use std::ptr::NonNull;
 
+use super::Error;
+
 #[derive(Debug)]
 pub struct TskBox<T> {
     tsk: NonNull<T>,
@@ -19,11 +21,15 @@ impl<T> TskBox<T> {
     pub fn new<F: Fn(*mut T) -> i32>(
         init: F,
         teardown: unsafe extern "C" fn(*mut T) -> i32,
-    ) -> Self {
+    ) -> Result<Self, Error> {
         // SAFETY: we will initialize it next
         let mut uninit = unsafe { Self::new_uninit(teardown) };
-        let _ = init(uninit.as_mut());
-        uninit
+        let rv = init(uninit.as_mut());
+        if rv < 0 {
+            Err(Error::Code(rv))
+        } else {
+            Ok(uninit)
+        }
     }
 
     // # Safety
@@ -136,7 +142,8 @@ fn test_miri() {
             0
         },
         teardown_x,
-    );
+    )
+    .unwrap();
 
     let _ = unsafe { TskBox::new_borrowed(&b) };
 
@@ -161,7 +168,8 @@ fn test_into_raw_miri() {
             0
         },
         teardown_x,
-    );
+    )
+    .unwrap();
 
     let p = unsafe { b.into_raw() };
 
@@ -187,6 +195,7 @@ fn test_table_collection_tskbox_shared_ptr() {
             super::bindings::tsk_table_collection_init(t, flags)
         },
         super::bindings::tsk_table_collection_free,
-    );
+    )
+    .unwrap();
     let _ = unsafe { TskBox::new_borrowed(&tables) };
 }

src/table_collection.rs

@@ -80,7 +80,7 @@ impl TableCollection {
                 expected: "sequence_length >= 0.0".to_string(),
             });
         }
-        let mut inner = LLTableCollection::new(sequence_length.into());
+        let mut inner = LLTableCollection::new(sequence_length.into())?;
         let views = crate::table_views::TableViews::new_from_ll_table_collection(&mut inner)?;
         Ok(Self {
             inner,
@@ -101,7 +101,7 @@ impl TableCollection {
 
     pub(crate) fn into_raw(self) -> Result<*mut ll_bindings::tsk_table_collection_t, TskitError> {
         let mut tables = self;
-        let mut temp = crate::sys::TableCollection::new(1.);
+        let mut temp = crate::sys::TableCollection::new(1.)?;
         std::mem::swap(&mut temp, &mut tables.inner);
         let ptr = temp.as_mut_ptr();
         std::mem::forget(temp);