fix: use correct return value in bcf_get_format and bcf_get_info_values (#398)

* fix: use correct return value in bcf_get_format and info

* test: check fs1 string vector length

* fix: sample count is 2 for test vcf file

* refactor: replace n with ret

* test: use shared buffer to test

* style: run cargo fmt

---------

Co-authored-by: Jialin Ma <[email protected]>

Author: Marlin-Na

src/bcf/mod.rs

@@ -1022,15 +1022,12 @@ mod tests {
                     .expect("Missing tag")[0],
                 format!("string{}", i + 1).as_bytes()
             );
-            println!(
-                "{}",
-                String::from_utf8_lossy(
-                    record
-                        .format(b"FS1")
-                        .string()
-                        .expect("Error reading string.")[0]
-                )
-            );
+            let fs1_str_vec = record
+                .format_shared_buffer(b"FS1", &mut buffer)
+                .string()
+                .expect("Error reading string.");
+            assert_eq!(fs1_str_vec.len(), 2);
+            println!("{}", String::from_utf8_lossy(fs1_str_vec[0]));
             assert_eq!(
                 record
                     .format(b"FS1")

src/bcf/record.rs

@@ -1251,7 +1251,7 @@ impl<'a, 'b, B: BorrowMut<Buffer> + Borrow<Buffer> + 'b> Info<'a, B> {
         str::from_utf8(self.tag).unwrap().to_owned()
     }
 
-    fn data(&mut self, data_type: u32) -> Result<Option<(usize, i32)>> {
+    fn data(&mut self, data_type: u32) -> Result<Option<i32>> {
         let mut n: i32 = self.buffer.borrow().len;
         let c_str = ffi::CString::new(self.tag).unwrap();
         let ret = unsafe {
@@ -1270,7 +1270,7 @@ impl<'a, 'b, B: BorrowMut<Buffer> + Borrow<Buffer> + 'b> Info<'a, B> {
             -1 => Err(Error::BcfUndefinedTag { tag: self.desc() }),
             -2 => Err(Error::BcfUnexpectedType { tag: self.desc() }),
             -3 => Ok(None),
-            ret => Ok(Some((n as usize, ret))),
+            ret => Ok(Some(ret)),
         }
     }
 
@@ -1284,9 +1284,10 @@ impl<'a, 'b, B: BorrowMut<Buffer> + Borrow<Buffer> + 'b> Info<'a, B> {
     /// memory.
     pub fn integer(mut self) -> Result<Option<BufferBacked<'b, &'b [i32], B>>> {
         self.data(htslib::BCF_HT_INT).map(|data| {
-            data.map(|(n, ret)| {
-                let values =
-                    unsafe { slice::from_raw_parts(self.buffer.borrow().inner as *const i32, n) };
+            data.map(|ret| {
+                let values = unsafe {
+                    slice::from_raw_parts(self.buffer.borrow().inner as *const i32, ret as usize)
+                };
                 BufferBacked::new(&values[..ret as usize], self.buffer)
             })
         })
@@ -1302,9 +1303,10 @@ impl<'a, 'b, B: BorrowMut<Buffer> + Borrow<Buffer> + 'b> Info<'a, B> {
     /// memory.
     pub fn float(mut self) -> Result<Option<BufferBacked<'b, &'b [f32], B>>> {
         self.data(htslib::BCF_HT_REAL).map(|data| {
-            data.map(|(n, ret)| {
-                let values =
-                    unsafe { slice::from_raw_parts(self.buffer.borrow().inner as *const f32, n) };
+            data.map(|ret| {
+                let values = unsafe {
+                    slice::from_raw_parts(self.buffer.borrow().inner as *const f32, ret as usize)
+                };
                 BufferBacked::new(&values[..ret as usize], self.buffer)
             })
         })
@@ -1313,7 +1315,7 @@ impl<'a, 'b, B: BorrowMut<Buffer> + Borrow<Buffer> + 'b> Info<'a, B> {
     /// Get flags from tag. `false` if not set.
     pub fn flag(&mut self) -> Result<bool> {
         self.data(htslib::BCF_HT_FLAG).map(|data| match data {
-            Some((_, ret)) => ret == 1,
+            Some(ret) => ret == 1,
             None => false,
         })
     }
@@ -1326,7 +1328,7 @@ impl<'a, 'b, B: BorrowMut<Buffer> + Borrow<Buffer> + 'b> Info<'a, B> {
     /// memory.
     pub fn string(mut self) -> Result<Option<BufferBacked<'b, Vec<&'b [u8]>, B>>> {
         self.data(htslib::BCF_HT_STR).map(|data| {
-            data.map(|(_, ret)| {
+            data.map(|ret| {
                 BufferBacked::new(
                     unsafe {
                         slice::from_raw_parts(self.buffer.borrow().inner as *const u8, ret as usize)
@@ -1402,7 +1404,7 @@ impl<'a, 'b, B: BorrowMut<Buffer> + Borrow<Buffer> + 'b> Format<'a, B> {
     }
 
     /// Read and decode format data into a given type.
-    fn data(&mut self, data_type: u32) -> Result<(usize, i32)> {
+    fn data(&mut self, data_type: u32) -> Result<i32> {
         let mut n: i32 = self.buffer.borrow().len;
         let c_str = ffi::CString::new(self.tag).unwrap();
         let ret = unsafe {
@@ -1423,7 +1425,7 @@ impl<'a, 'b, B: BorrowMut<Buffer> + Borrow<Buffer> + 'b> Format<'a, B> {
                 tag: self.desc(),
                 record: self.record.desc(),
             }),
-            ret => Ok((n as usize, ret)),
+            ret => Ok(ret),
         }
     }
 
@@ -1434,12 +1436,17 @@ impl<'a, 'b, B: BorrowMut<Buffer> + Borrow<Buffer> + 'b> Format<'a, B> {
     /// the BufferBacked object is already dropped, you will access unallocated
     /// memory.
     pub fn integer(mut self) -> Result<BufferBacked<'b, Vec<&'b [i32]>, B>> {
-        self.data(htslib::BCF_HT_INT).map(|(n, _)| {
+        self.data(htslib::BCF_HT_INT).map(|ret| {
             BufferBacked::new(
-                unsafe { slice::from_raw_parts(self.buffer.borrow_mut().inner as *const i32, n) }
-                    .chunks(self.values_per_sample())
-                    .map(|s| trim_slice(s))
-                    .collect(),
+                unsafe {
+                    slice::from_raw_parts(
+                        self.buffer.borrow_mut().inner as *const i32,
+                        ret as usize,
+                    )
+                }
+                .chunks(self.values_per_sample())
+                .map(|s| trim_slice(s))
+                .collect(),
                 self.buffer,
             )
         })
@@ -1452,12 +1459,17 @@ impl<'a, 'b, B: BorrowMut<Buffer> + Borrow<Buffer> + 'b> Format<'a, B> {
     /// the BufferBacked object is already dropped, you will access unallocated
     /// memory.
     pub fn float(mut self) -> Result<BufferBacked<'b, Vec<&'b [f32]>, B>> {
-        self.data(htslib::BCF_HT_REAL).map(|(n, _)| {
+        self.data(htslib::BCF_HT_REAL).map(|ret| {
             BufferBacked::new(
-                unsafe { slice::from_raw_parts(self.buffer.borrow_mut().inner as *const f32, n) }
-                    .chunks(self.values_per_sample())
-                    .map(|s| trim_slice(s))
-                    .collect(),
+                unsafe {
+                    slice::from_raw_parts(
+                        self.buffer.borrow_mut().inner as *const f32,
+                        ret as usize,
+                    )
+                }
+                .chunks(self.values_per_sample())
+                .map(|s| trim_slice(s))
+                .collect(),
                 self.buffer,
             )
         })
@@ -1470,17 +1482,19 @@ impl<'a, 'b, B: BorrowMut<Buffer> + Borrow<Buffer> + 'b> Format<'a, B> {
     /// the BufferBacked object is already dropped, you will access unallocated
     /// memory.
     pub fn string(mut self) -> Result<BufferBacked<'b, Vec<&'b [u8]>, B>> {
-        self.data(htslib::BCF_HT_STR).map(|(n, _)| {
+        self.data(htslib::BCF_HT_STR).map(|ret| {
             BufferBacked::new(
-                unsafe { slice::from_raw_parts(self.buffer.borrow_mut().inner as *const u8, n) }
-                    .chunks(self.values_per_sample())
-                    .map(|s| {
-                        // stop at zero character
-                        s.split(|c| *c == 0u8)
-                            .next()
-                            .expect("Bug: returned string should not be empty.")
-                    })
-                    .collect(),
+                unsafe {
+                    slice::from_raw_parts(self.buffer.borrow_mut().inner as *const u8, ret as usize)
+                }
+                .chunks(self.values_per_sample())
+                .map(|s| {
+                    // stop at zero character
+                    s.split(|c| *c == 0u8)
+                        .next()
+                        .expect("Bug: returned string should not be empty.")
+                })
+                .collect(),
                 self.buffer,
             )
         })