From bb31c5f52e9e7366d060ece4f1ad8654af305226 Mon Sep 17 00:00:00 2001 From: Stefan Reiter Date: Mon, 2 Nov 2020 12:05:17 +0100 Subject: [PATCH 1/2] add version details to simple API And extend policy example by printing short description for each version found, to test it. --- examples/policy.rs | 5 ++++- src/simple.rs | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/examples/policy.rs b/examples/policy.rs index a5d1371..5ea9af2 100644 --- a/examples/policy.rs +++ b/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 { diff --git a/src/simple.rs b/src/simple.rs index c16b3d4..64eba05 100644 --- a/src/simple.rs +++ b/src/simple.rs @@ -36,6 +36,25 @@ impl fmt::Display for BinaryPackage { } } +#[derive(Clone, Debug)] +pub struct VersionDetails { + pub short_desc: Option, + pub long_desc: Option, + pub maintainer: Option, + pub homepage: Option, +} + +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, } } } From 06795e39f2e7dfe08456a59337ce5c70d4888e52 Mon Sep 17 00:00:00 2001 From: Stefan Reiter Date: Mon, 2 Nov 2020 12:18:24 +0100 Subject: [PATCH 2/2] correctly free data allocated by PVerFileParser Adjust return type of ver_iter_* to be *const c_char, as that data does not need manual freeing. ver_file_parser_* function do require it (they use to_c_string) and the parser itself also needs to be freed. Add a SafeVerFileParser type to free the parser on drop, and free the data returned by the parser ASAP after cloning into a Rust String. Data is allocated with C++ 'new', so we add custom functions to call 'delete' instead of relying on libc::free to avoid any potential issues. --- apt-pkg-c/lib.cpp | 16 +++++++++++++++- src/raw.rs | 24 +++++++++++++----------- src/sane.rs | 43 +++++++++++++++++++++++++++++++++++++------ 3 files changed, 65 insertions(+), 18 deletions(-) diff --git a/apt-pkg-c/lib.cpp b/apt-pkg-c/lib.cpp index 22dc5f7..e2c153a 100644 --- a/apt-pkg-c/lib.cpp +++ b/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(); } diff --git a/src/raw.rs b/src/raw.rs index 28bcad9..2c03692 100644 --- a/src/raw.rs +++ b/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); diff --git a/src/sane.rs b/src/sane.rs index f0eecf5..49804b7 100644 --- a/src/sane.rs +++ b/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 { + 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 { - 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 { - 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 { - 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 { - unsafe { make_owned_ascii_string(raw::ver_file_parser_homepage(self.parser)) } + unsafe { self.retrieve_str(raw::ver_file_parser_homepage) } } }