fix leaked header mutability (#181)

htslib may modify `cigar_tab` in header as records are parsed,
also fix misc warnings from compiler

Author: Lance Hepler

src/bam/mod.rs

@@ -92,6 +92,9 @@ pub trait Read: Sized {
     /// Return the header.
     fn header(&self) -> &HeaderView;
 
+    /// Return the header, mutable.
+    fn header_mut(&mut self) -> &mut HeaderView;
+
     /// Seek to the given virtual offset in the file
     fn seek(&mut self, offset: i64) -> Result<()> {
         let htsfile = unsafe { self.htsfile().as_ref() }.expect("bug: null pointer to htsFile");
@@ -198,8 +201,8 @@ impl Reader {
         data: *mut ::std::os::raw::c_void,
         record: *mut htslib::bam1_t,
     ) -> i32 {
-        let _self = unsafe { &*(data as *mut Self) };
-        unsafe { htslib::sam_read1(_self.htsfile(), &mut _self.header().inner(), record) }
+        let mut _self = unsafe { (data as *mut Self).as_mut().unwrap() };
+        unsafe { htslib::sam_read1(_self.htsfile(), _self.header_mut().inner_mut(), record) }
     }
 
     /// Iterator over the records between the (optional) virtual offsets `start` and `end`
@@ -258,7 +261,7 @@ impl Read for Reader {
         match unsafe {
             htslib::sam_read1(
                 self.htsfile,
-                &mut self.header.inner(),
+                self.header.inner_mut(),
                 record.inner_ptr_mut(),
             )
         } {
@@ -295,6 +298,10 @@ impl Read for Reader {
     fn header(&self) -> &HeaderView {
         &self.header
     }
+
+    fn header_mut(&mut self) -> &mut HeaderView {
+        &mut self.header
+    }
 }
 
 impl Drop for Reader {
@@ -412,7 +419,7 @@ impl IndexedReader {
         }
         let rstr = ffi::CString::new(region).unwrap();
         let rptr = rstr.as_ptr();
-        let itr = unsafe { htslib::sam_itr_querys(self.idx, &mut self.header.inner(), rptr) };
+        let itr = unsafe { htslib::sam_itr_querys(self.idx, self.header.inner_mut(), rptr) };
         if itr.is_null() {
             self.itr = None;
             Err(Error::Fetch)
@@ -426,10 +433,10 @@ impl IndexedReader {
         data: *mut ::std::os::raw::c_void,
         record: *mut htslib::bam1_t,
     ) -> i32 {
-        let _self = unsafe { &*(data as *mut Self) };
+        let _self = unsafe { (data as *mut Self).as_mut().unwrap() };
         match _self.itr {
             Some(itr) => itr_next(_self.htsfile, itr, record), // read fetched region
-            None => unsafe { htslib::sam_read1(_self.htsfile, &mut _self.header.inner(), record) }, // ordinary reading
+            None => unsafe { htslib::sam_read1(_self.htsfile, _self.header.inner_mut(), record) }, // ordinary reading
         }
     }
 
@@ -484,6 +491,10 @@ impl Read for IndexedReader {
     fn header(&self) -> &HeaderView {
         &self.header
     }
+
+    fn header_mut(&mut self) -> &mut HeaderView {
+        &mut self.header
+    }
 }
 
 impl Drop for IndexedReader {
@@ -612,7 +623,7 @@ impl Writer {
     ///
     /// * `record` - the record to write
     pub fn write(&mut self, record: &record::Record) -> Result<()> {
-        if unsafe { htslib::sam_write1(self.f, &self.header.inner(), record.inner_ptr()) } == -1 {
+        if unsafe { htslib::sam_write1(self.f, self.header.inner(), record.inner_ptr()) } == -1 {
             Err(Error::Write)
         } else {
             Ok(())
@@ -819,8 +830,8 @@ impl HeaderView {
     }
 
     #[inline]
-    pub fn inner(&self) -> htslib::bam_hdr_t {
-        unsafe { (*self.inner) }
+    pub fn inner(&self) -> &htslib::bam_hdr_t {
+        unsafe { self.inner.as_ref().unwrap() }
     }
 
     #[inline]
@@ -829,9 +840,14 @@ impl HeaderView {
         self.inner
     }
 
+    #[inline]
+    pub fn inner_mut(&mut self) -> &mut htslib::bam_hdr_t {
+        unsafe { self.inner.as_mut().unwrap() }
+    }
+
     #[inline]
     // Mutable pointer to bam_hdr_t struct
-    pub fn inner_ptr_mut(&self) -> *mut htslib::bam_hdr_t {
+    pub fn inner_ptr_mut(&mut self) -> *mut htslib::bam_hdr_t {
         self.inner
     }
 
@@ -1249,12 +1265,12 @@ CCCCCCCCCCCCCCCCCCC"[..],
                 .push_tag(b"SN", &"1")
                 .push_tag(b"LN", &10000000),
         );
-        let header = HeaderView::from_header(&_header);
+        let mut header = HeaderView::from_header(&_header);
 
         let line =
             b"blah1	0	1	1	255	1M	*	0	0	A	F	CB:Z:AAAA-1	UR:Z:AAAA	UB:Z:AAAA	GX:Z:G1	xf:i:1	fx:Z:G1\tli:i:0\ttf:Z:cC";
 
-        let mut rec = Record::from_sam(&header, line).unwrap();
+        let mut rec = Record::from_sam(&mut header, line).unwrap();
         assert_eq!(rec.qname(), b"blah1");
         rec.set_qname(b"r0");
         assert_eq!(rec.qname(), b"r0");
@@ -1492,7 +1508,7 @@ CCCCCCCCCCCCCCCCCCC"[..],
         let sam_recs: Vec<Record> = sam
             .split(|x| *x == b'\n')
             .filter(|x| x.len() > 0 && x[0] != b'@')
-            .map(|line| Record::from_sam(rdr.header(), line).unwrap())
+            .map(|line| Record::from_sam(rdr.header_mut(), line).unwrap())
             .collect();
 
         for (b1, s1) in bam_recs.iter().zip(sam_recs.iter()) {

src/bam/record.rs

@@ -131,7 +131,7 @@ impl Record {
     }
 
     // Create a BAM record from a line SAM text. SAM slice need not be 0-terminated.
-    pub fn from_sam(header_view: &HeaderView, sam: &[u8]) -> Result<Record> {
+    pub fn from_sam(header_view: &mut HeaderView, sam: &[u8]) -> Result<Record> {
         let mut record = Self::new();
 
         let mut sam_copy = Vec::with_capacity(sam.len() + 1);

src/bcf/mod.rs

@@ -953,7 +953,7 @@ mod tests {
             .ok()
             .expect("Error opening file.");
         let expected = ["./1", "1|1", "0/1", "0|1", "1|.", "1/1"];
-        for (rec, exp_gt) in vcf.records().zip(expected.into_iter()) {
+        for (rec, exp_gt) in vcf.records().zip(expected.iter()) {
             let mut rec = rec.expect("Error reading record.");
             let genotypes = rec.genotypes().expect("Error reading genotypes");
             assert_eq!(&format!("{}", genotypes.get(0)), exp_gt);
@@ -1327,7 +1327,7 @@ mod tests {
     fn test_multi_string_info_tag() {
         let mut reader = Reader::from_path("test/test-info-multi-string.vcf").unwrap();
         let mut rec = reader.empty_record();
-        reader.read(&mut rec);
+        let _ = reader.read(&mut rec);
 
         assert_eq!(rec.info(b"ANN").string().unwrap().unwrap().len(), 14);
     }
@@ -1336,7 +1336,7 @@ mod tests {
     fn test_multi_string_info_tag_number_a() {
         let mut reader = Reader::from_path("test/test-info-multi-string-number=A.vcf").unwrap();
         let mut rec = reader.empty_record();
-        reader.read(&mut rec);
+        let _ = reader.read(&mut rec);
 
         assert_eq!(rec.info(b"X").string().unwrap().unwrap().len(), 2);
     }