Merge branch 'master' into use-arcs-in-records

Author: johanneskoester

src/bam/buffer.rs

@@ -5,8 +5,8 @@
 
 use std::collections::{vec_deque, VecDeque};
 use std::mem;
-use std::rc::Rc;
 use std::str;
+use std::sync::Arc;
 
 use crate::bam;
 use crate::bam::Read;
@@ -19,11 +19,11 @@ use crate::errors::{Error, Result};
 #[derive(Debug)]
 pub struct RecordBuffer {
     reader: bam::IndexedReader,
-    inner: VecDeque<Rc<bam::Record>>,
-    overflow: Option<Rc<bam::Record>>,
+    inner: VecDeque<Arc<bam::Record>>,
+    overflow: Option<Arc<bam::Record>>,
     cache_cigar: bool,
     min_refetch_distance: u64,
-    buffer_record: Rc<bam::Record>,
+    buffer_record: Arc<bam::Record>,
     start_pos: Option<u64>,
 }
 
@@ -44,7 +44,7 @@ impl RecordBuffer {
             overflow: None,
             cache_cigar,
             min_refetch_distance: 1,
-            buffer_record: Rc::new(bam::Record::new()),
+            buffer_record: Arc::new(bam::Record::new()),
             start_pos: Some(0),
         }
     }
@@ -113,7 +113,7 @@ impl RecordBuffer {
             loop {
                 match self
                     .reader
-                    .read(Rc::get_mut(&mut self.buffer_record).unwrap())
+                    .read(Arc::get_mut(&mut self.buffer_record).unwrap())
                 {
                     None => break,
                     Some(res) => res?,
@@ -132,10 +132,11 @@ impl RecordBuffer {
 
                 // Record is kept, do not reuse it for next iteration
                 // and thus create a new one.
-                let mut record = mem::replace(&mut self.buffer_record, Rc::new(bam::Record::new()));
+                let mut record =
+                    mem::replace(&mut self.buffer_record, Arc::new(bam::Record::new()));
 
                 if self.cache_cigar {
-                    Rc::get_mut(&mut record).unwrap().cache_cigar();
+                    Arc::get_mut(&mut record).unwrap().cache_cigar();
                 }
 
                 if pos >= end as i64 {
@@ -157,12 +158,12 @@ impl RecordBuffer {
     }
 
     /// Iterate over records that have been fetched with `fetch`.
-    pub fn iter(&'_ self) -> vec_deque::Iter<'_, Rc<bam::Record>> {
+    pub fn iter(&self) -> vec_deque::Iter<'_, Arc<bam::Record>> {
         self.inner.iter()
     }
 
     /// Iterate over mutable references to records that have been fetched with `fetch`.
-    pub fn iter_mut(&'_ mut self) -> vec_deque::IterMut<'_, Rc<bam::Record>> {
+    pub fn iter_mut(&mut self) -> vec_deque::IterMut<'_, Arc<bam::Record>> {
         self.inner.iter_mut()
     }
 

src/bam/mod.rs

@@ -1692,6 +1692,7 @@ CCCCCCCCCCCCCCCCCCC"[..],
             // fix qual offset
             let qual: Vec<u8> = quals[i].iter().map(|&q| q - 33).collect();
             assert_eq!(rec.qual(), &qual[..]);
+            assert_eq!(rec.aux(b"X"), Err(Error::BamAuxStringError));
             assert_eq!(rec.aux(b"NotAvailableAux"), Err(Error::BamAuxTagNotFound));
         }
 
@@ -2006,7 +2007,8 @@ CCCCCCCCCCCCCCCCCCC"[..],
                 rec.remove_aux(b"YT").unwrap();
             }
 
-            assert!(rec.remove_aux(b"ab").is_err());
+            assert_eq!(rec.remove_aux(b"X"), Err(Error::BamAuxStringError));
+            assert_eq!(rec.remove_aux(b"ab"), Err(Error::BamAuxTagNotFound));
 
             assert_eq!(rec.aux(b"XS"), Err(Error::BamAuxTagNotFound));
             assert_eq!(rec.aux(b"YT"), Err(Error::BamAuxTagNotFound));

src/bam/record.rs

@@ -644,11 +644,13 @@ impl Record {
     /// Only the first two bytes of a given tag are used for the look-up of a field.
     /// See [`Aux`] for more details.
     pub fn aux(&self, tag: &[u8]) -> Result<Aux<'_>> {
-        let c_str = ffi::CString::new(tag).map_err(|_| Error::BamAuxStringError)?;
+        if tag.len() < 2 {
+            return Err(Error::BamAuxStringError);
+        }
         let aux = unsafe {
             htslib::bam_aux_get(
                 &self.inner as *const htslib::bam1_t,
-                c_str.as_ptr() as *mut c_char,
+                tag.as_ptr() as *const c_char,
             )
         };
         unsafe { Self::read_aux_field(aux).map(|(aux_field, _length)| aux_field) }
@@ -1224,11 +1226,13 @@ impl Record {
 
     // Delete auxiliary tag.
     pub fn remove_aux(&mut self, tag: &[u8]) -> Result<()> {
-        let c_str = ffi::CString::new(tag).map_err(|_| Error::BamAuxStringError)?;
+        if tag.len() < 2 {
+            return Err(Error::BamAuxStringError);
+        }
         let aux = unsafe {
             htslib::bam_aux_get(
                 &self.inner as *const htslib::bam1_t,
-                c_str.as_ptr() as *mut c_char,
+                tag.as_ptr() as *const c_char,
             )
         };
         unsafe {

src/bcf/header.rs

@@ -519,15 +519,8 @@ impl HeaderView {
     /// Create an empty record using this header view.
     ///
     /// The record can be reused multiple times.
-    ///
-    /// # Performance
-    ///
-    /// This is quite slow and resource-intensive as it clones the actual
-    /// header, instead of the reference to it. Therefore, whenever possible
-    /// / feasible you should use [`Record::new`](crate::bcf::Record::new) with
-    /// a reference to your header.
-    pub fn empty_record(&self) -> crate::bcf::Record {
-        crate::bcf::Record::new(Arc::new(self.clone()))
+    pub fn empty_record(self: &Arc<Self>) -> crate::bcf::Record {
+        crate::bcf::Record::new(self.clone())
     }
 }