From bdea7de6f29d027bae305ab60d1c294d7d9cf30f Mon Sep 17 00:00:00 2001
From: Chris Saunders <csaunders@pacificbiosciences.com>
Date: Mon, 21 Oct 2024 11:09:59 -0700
Subject: [PATCH] Revert to Record::to_vcf_string method

Added return value checking on vcf_format, which also suggests that the
methods should return a Result and shound't be implemented directly as
a Display trait.
---
 src/bcf/record.rs | 69 ++++++++++++++++++++++++++++++++---------------
 src/errors.rs     |  2 ++
 2 files changed, 49 insertions(+), 22 deletions(-)

diff --git a/src/bcf/record.rs b/src/bcf/record.rs
index c0ddbf0ad..e84a6273f 100644
--- a/src/bcf/record.rs
+++ b/src/bcf/record.rs
@@ -1088,6 +1088,38 @@ impl Record {
         }
         "".to_owned()
     }
+
+    /// Convert to VCF String
+    ///
+    /// Intended for debug only. Use Writer for efficient VCF output.
+    ///
+    pub fn to_vcf_string(&self) -> Result<String> {
+        let mut buf = htslib::kstring_t {
+            l: 0,
+            m: 0,
+            s: ptr::null_mut(),
+        };
+        let ret = unsafe { htslib::vcf_format(self.header().inner, self.inner, &mut buf) };
+
+        if ret < 0 {
+            if !buf.s.is_null() {
+                unsafe {
+                    libc::free(buf.s as *mut libc::c_void);
+                }
+            }
+            return Err(Error::BcfToString);
+        }
+
+        let vcf_str = unsafe {
+            let vcf_str = String::from(ffi::CStr::from_ptr(buf.s).to_str().unwrap());
+            if !buf.s.is_null() {
+                libc::free(buf.s as *mut libc::c_void);
+            }
+            vcf_str
+        };
+
+        Ok(vcf_str)
+    }
 }
 
 impl Clone for Record {
@@ -1237,25 +1269,6 @@ unsafe impl Send for Record {}
 
 unsafe impl Sync for Record {}
 
-impl fmt::Display for Record {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        let mut buf = htslib::kstring_t {
-            l: 0,
-            m: 0,
-            s: ptr::null_mut(),
-        };
-        let vcf_str = unsafe {
-            htslib::vcf_format(self.header().inner, self.inner, &mut buf);
-            let vcf_str = String::from(ffi::CStr::from_ptr(buf.s).to_str().unwrap());
-            if !buf.s.is_null() {
-                libc::free(buf.s as *mut libc::c_void);
-            }
-            vcf_str
-        };
-        write!(f, "{vcf_str}")
-    }
-}
-
 /// Info tag representation.
 #[derive(Debug)]
 pub struct Info<'a, B: BorrowMut<Buffer> + Borrow<Buffer>> {
@@ -1735,7 +1748,17 @@ mod tests {
     }
 
     #[test]
-    fn test_record_display_trait() {
+    fn test_record_to_vcf_string_err() {
+        let tmp = NamedTempFile::new().unwrap();
+        let path = tmp.path();
+        let header = Header::new();
+        let vcf = Writer::from_path(path, &header, true, Format::Vcf).unwrap();
+        let record = vcf.empty_record();
+        assert!(record.to_vcf_string().is_err());
+    }
+
+    #[test]
+    fn test_record_to_vcf_string() {
         let tmp = NamedTempFile::new().unwrap();
         let path = tmp.path();
         let mut header = Header::new();
@@ -1744,7 +1767,9 @@ mod tests {
         let vcf = Writer::from_path(path, &header, true, Format::Vcf).unwrap();
         let mut record = vcf.empty_record();
         record.push_filter("foo".as_bytes()).unwrap();
-        let s = format!("{record}");
-        assert_eq!(s, "chr1\t1\t.\t.\t.\t0\tfoo\t.\n");
+        assert_eq!(
+            record.to_vcf_string().unwrap(),
+            "chr1\t1\t.\t.\t.\t0\tfoo\t.\n"
+        );
     }
 }
diff --git a/src/errors.rs b/src/errors.rs
index 2e89854f0..82693a381 100644
--- a/src/errors.rs
+++ b/src/errors.rs
@@ -129,6 +129,8 @@ pub enum Error {
     BcfSetValues,
     #[error("failed to remove alleles in BCF/VCF record")]
     BcfRemoveAlleles,
+    #[error("failed to render BCF record as string")]
+    BcfToString,
 
     #[error("invalid compression level {level}")]
     BgzfInvalidCompressionLevel { level: i8 },