close #6: correctly free allocated memory

apt-pkg-c/lib.cpp

@@ -146,7 +146,12 @@ extern "C" {
     const char *ver_file_parser_maintainer(PVerFileParser *parser);
     const char *ver_file_parser_homepage(PVerFileParser *parser);
 
-    // ver_file_iter has no accessors, only the creation of pkg_file_iter
+    // parser data needs manual freeing
+    void ver_file_parser_free_str(char *ptr);
+    void ver_file_parser_free(PVerFileParser *parser);
+
+    // PVerFileIterator has no accessors, only the creation of PPkgFileIterator
+    // and PVerFileParser
 
 
     // pkg_file_iter creation
@@ -369,6 +374,7 @@ PVerFileParser *ver_file_iter_get_parser(PVerFileIterator *wrapper) {
     return parser;
 }
 
+// must be freed with ver_file_parser_free_str
 const char *to_c_string(std::string s) {
     char *cstr = new char[s.length()+1];
     std::strcpy(cstr, s.c_str());
@@ -395,6 +401,14 @@ const char *ver_file_parser_homepage(PVerFileParser *parser) {
     return to_c_string(hp);
 }
 
+void ver_file_parser_free_str(char *ptr) {
+    delete ptr;
+}
+
+void ver_file_parser_free(PVerFileParser *parser) {
+    delete parser;
+}
+
 bool ver_file_iter_end(PVerFileIterator *wrapper) {
     return wrapper->iterator.end();
 }

examples/policy.rs

@@ -42,8 +42,11 @@ fn main() {
             #[cfg(feature = "ye-olde-apt")]
             println!(" {} {}", marker, version.version,);
 
+            println!("       {} {}", "Desc:",
+                version.details.short_desc.unwrap_or("-".to_owned()));
+
             for origin in origins {
-                println!("       {:4} {}", "XXX", origin);
+                println!("       {} {}", "Orig:", origin);
             }
         }
     } else {

src/raw.rs

@@ -1,7 +1,7 @@
 /// In general:
 ///  * `*mut c_void` are to be released by the appropriate function
 ///  * `*const c_chars` are short-term borrows
-///  * `*mut c_chars` are to be freed by `libc::free`.
+///  * `*mut c_chars` are to be freed by their associated 'free' function
 use std::sync::Mutex;
 
 use lazy_static::lazy_static;
@@ -65,16 +65,16 @@ extern "C" {
     // Version accessors
     // =================
 
-    pub fn ver_iter_version(iterator: PVerIterator) -> *mut c_char;
-    pub fn ver_iter_section(iterator: PVerIterator) -> *mut c_char;
+    pub fn ver_iter_version(iterator: PVerIterator) -> *const c_char;
+    pub fn ver_iter_section(iterator: PVerIterator) -> *const c_char;
 
     #[cfg(not(feature = "ye-olde-apt"))]
-    pub fn ver_iter_source_package(iterator: PVerIterator) -> *mut c_char;
+    pub fn ver_iter_source_package(iterator: PVerIterator) -> *const c_char;
 
     #[cfg(not(feature = "ye-olde-apt"))]
-    pub fn ver_iter_source_version(iterator: PVerIterator) -> *mut c_char;
-    pub fn ver_iter_arch(iterator: PVerIterator) -> *mut c_char;
-    pub fn ver_iter_priority_type(iterator: PVerIterator) -> *mut c_char;
+    pub fn ver_iter_source_version(iterator: PVerIterator) -> *const c_char;
+    pub fn ver_iter_arch(iterator: PVerIterator) -> *const c_char;
+    pub fn ver_iter_priority_type(iterator: PVerIterator) -> *const c_char;
 
     #[cfg(not(feature = "ye-olde-apt"))]
     pub fn ver_iter_priority(iterator: PVerIterator) -> i32;
@@ -103,10 +103,12 @@ extern "C" {
     pub fn ver_file_iter_end(iterator: PVerFileIterator) -> bool;
 
     pub fn ver_file_iter_get_parser(iterator: PVerFileIterator) -> PVerFileParser;
-    pub fn ver_file_parser_short_desc(parser: PVerFileParser) -> *const c_char;
-    pub fn ver_file_parser_long_desc(parser: PVerFileParser) -> *const c_char;
-    pub fn ver_file_parser_maintainer(parser: PVerFileParser) -> *const c_char;
-    pub fn ver_file_parser_homepage(parser: PVerFileParser) -> *const c_char;
+    pub fn ver_file_parser_short_desc(parser: PVerFileParser) -> *mut c_char;
+    pub fn ver_file_parser_long_desc(parser: PVerFileParser) -> *mut c_char;
+    pub fn ver_file_parser_maintainer(parser: PVerFileParser) -> *mut c_char;
+    pub fn ver_file_parser_homepage(parser: PVerFileParser) -> *mut c_char;
+    pub fn ver_file_parser_free_str(ptr: *mut c_char);
+    pub fn ver_file_parser_free(parser: PVerFileParser);
 
     pub fn ver_file_iter_pkg_file_iter(iterator: PVerFileIterator) -> PPkgFileIterator;
     pub fn pkg_file_iter_release(iterator: PPkgFileIterator);

src/sane.rs

@@ -379,12 +379,31 @@ pub struct VerFileIterator<'c> {
     ptr: raw::PVerFileIterator,
 }
 
+struct SafeVerFileParser {
+    ptr: raw::PVerFileParser,
+}
+
+impl SafeVerFileParser {
+    fn new(ver_file: raw::PVerFileIterator) -> Self {
+        let ptr = unsafe { raw::ver_file_iter_get_parser(ver_file) };
+        Self {
+            ptr,
+        }
+    }
+}
+
+impl Drop for SafeVerFileParser {
+    fn drop(&mut self) {
+        unsafe { raw::ver_file_parser_free(self.ptr); }
+    }
+}
+
 // TODO: could this be a ref to the iterator?
 // TODO: Can't get the lifetimes to work.
 pub struct VerFileView<'c> {
     cache: PhantomData<&'c MutexGuard<'c, raw::CacheHolder>>,
     ptr: raw::PVerFileIterator,
-    parser: raw::PVerFileParser,
+    parser: SafeVerFileParser,
 }
 
 impl<'c> RawIterator for VerFileIterator<'c> {
@@ -401,7 +420,7 @@ impl<'c> RawIterator for VerFileIterator<'c> {
     fn as_view(&self) -> Self::View {
         assert!(!self.is_end());
 
-        let parser = unsafe { raw::ver_file_iter_get_parser(self.ptr) };
+        let parser = SafeVerFileParser::new(self.ptr);
 
         VerFileView {
             ptr: self.ptr,
@@ -426,20 +445,32 @@ impl<'c> VerFileView<'c> {
         }
     }
 
+    unsafe fn retrieve_str(
+        &self,
+        f: unsafe extern "C" fn(raw::PVerFileParser) -> *mut libc::c_char
+    ) -> Option<String> {
+        let ptr = f(self.parser.ptr);
+        let ret = make_owned_ascii_string(ptr);
+        if !ptr.is_null() {
+            raw::ver_file_parser_free_str(ptr);
+        }
+        ret
+    }
+
     pub fn short_desc(&self) -> Option<String> {
-        unsafe { make_owned_ascii_string(raw::ver_file_parser_short_desc(self.parser)) }
+        unsafe { self.retrieve_str(raw::ver_file_parser_short_desc) }
     }
 
     pub fn long_desc(&self) -> Option<String> {
-        unsafe { make_owned_ascii_string(raw::ver_file_parser_long_desc(self.parser)) }
+        unsafe { self.retrieve_str(raw::ver_file_parser_long_desc) }
     }
 
     pub fn maintainer(&self) -> Option<String> {
-        unsafe { make_owned_ascii_string(raw::ver_file_parser_maintainer(self.parser)) }
+        unsafe { self.retrieve_str(raw::ver_file_parser_maintainer) }
     }
 
     pub fn homepage(&self) -> Option<String> {
-        unsafe { make_owned_ascii_string(raw::ver_file_parser_homepage(self.parser)) }
+        unsafe { self.retrieve_str(raw::ver_file_parser_homepage) }
     }
 }
 

src/simple.rs

@@ -36,6 +36,25 @@ impl fmt::Display for BinaryPackage {
     }
 }
 
+#[derive(Clone, Debug)]
+pub struct VersionDetails {
+    pub short_desc: Option<String>,
+    pub long_desc: Option<String>,
+    pub maintainer: Option<String>,
+    pub homepage: Option<String>,
+}
+
+impl Default for VersionDetails {
+    fn default() -> Self {
+        Self {
+            short_desc: None,
+            long_desc: None,
+            maintainer: None,
+            homepage: None,
+        }
+    }
+}
+
 #[derive(Clone, Debug)]
 pub struct Version {
     pub version: String,
@@ -48,10 +67,24 @@ pub struct Version {
     pub source_version: String,
     #[cfg(not(feature = "ye-olde-apt"))]
     pub priority: i32,
+
+    pub details: VersionDetails,
 }
 
 impl Version {
     pub fn new(view: &sane::VerView) -> Self {
+        // assume there is either zero or only one set of details per version
+        let details = if let Some(ver_file) = view.origin_iter().next() {
+            VersionDetails {
+                short_desc: ver_file.short_desc(),
+                long_desc: ver_file.long_desc(),
+                maintainer: ver_file.maintainer(),
+                homepage: ver_file.homepage(),
+            }
+        } else {
+            Default::default()
+        };
+
         Version {
             version: view.version(),
             arch: view.arch(),
@@ -62,6 +95,7 @@ impl Version {
             source_version: view.source_version(),
             #[cfg(not(feature = "ye-olde-apt"))]
             priority: view.priority(),
+            details,
         }
     }
 }