From 1317eea0559311db508c43430f22a6c328f5f615 Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Mon, 5 May 2025 11:45:50 +0100 Subject: [PATCH] Use Rust Filename comparisons for C++ Filename operators To prevent inconsistencies between implementations. This means allocating new boxed Filenames for each comparison, but an extra string copy is avoided by making the C++ operators friends of the C++ class (which is ABI-safe). --- Cargo.lock | 1 - cpp/Cargo.toml | 1 - cpp/include/loot/metadata/filename.h | 4 ++++ cpp/src/api/metadata/filename.cpp | 6 ++++-- cpp/src/lib.rs | 23 ++++++++++++----------- cpp/src/metadata.rs | 2 +- 6 files changed, 21 insertions(+), 16 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 697c9934..163ae5f6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -573,7 +573,6 @@ dependencies = [ "delegate", "libloot", "libloot-ffi-errors", - "unicase", ] [[package]] diff --git a/cpp/Cargo.toml b/cpp/Cargo.toml index c22adf5d..827ad2ee 100644 --- a/cpp/Cargo.toml +++ b/cpp/Cargo.toml @@ -9,7 +9,6 @@ cxx = { version = "1.0", features = ["c++17"] } delegate = "0.13.2" libloot = { path = ".." } libloot-ffi-errors = { path = "../ffi-errors" } -unicase = "2.8.1" [build-dependencies] cxx-build = "1.0" diff --git a/cpp/include/loot/metadata/filename.h b/cpp/include/loot/metadata/filename.h index 3c7fc9c9..81584fb5 100644 --- a/cpp/include/loot/metadata/filename.h +++ b/cpp/include/loot/metadata/filename.h @@ -54,6 +54,10 @@ public: private: std::string filename_; + + LOOT_API friend bool operator==(const Filename& lhs, const Filename& rhs); + + LOOT_API friend bool operator<(const Filename& lhs, const Filename& rhs); }; /** diff --git a/cpp/src/api/metadata/filename.cpp b/cpp/src/api/metadata/filename.cpp index 9d24d90e..e05c2b8e 100644 --- a/cpp/src/api/metadata/filename.cpp +++ b/cpp/src/api/metadata/filename.cpp @@ -35,7 +35,8 @@ Filename::Filename(std::string_view filename) : filename_(filename) {} Filename::operator std::string() const { return filename_; } bool operator==(const Filename& lhs, const Filename& rhs) { - return loot::rust::compare_filenames(std::string(lhs), std::string(rhs)) == 0; + return loot::rust::new_filename(lhs.filename_) + ->eq(*loot::rust::new_filename(rhs.filename_)); } bool operator!=(const Filename& lhs, const Filename& rhs) { @@ -43,7 +44,8 @@ bool operator!=(const Filename& lhs, const Filename& rhs) { } bool operator<(const Filename& lhs, const Filename& rhs) { - return loot::rust::compare_filenames(std::string(lhs), std::string(rhs)) < 0; + return loot::rust::new_filename(lhs.filename_) + ->lt(*loot::rust::new_filename(rhs.filename_)); } bool operator>(const Filename& lhs, const Filename& rhs) { return rhs < lhs; } diff --git a/cpp/src/lib.rs b/cpp/src/lib.rs index b1e3f141..6d33bcb4 100644 --- a/cpp/src/lib.rs +++ b/cpp/src/lib.rs @@ -119,7 +119,6 @@ use std::{ ffi::{CString, c_char, c_uchar, c_uint, c_void}, sync::{Mutex, atomic::AtomicPtr}, }; -use unicase::UniCase; use libloot::set_logging_callback; pub use libloot::{is_compatible, libloot_revision, libloot_version}; @@ -181,14 +180,6 @@ pub type OptionalPluginMetadata = Optional; pub type OptionalCrc = Optional; -fn compare_filenames(lhs: &str, rhs: &str) -> i8 { - match UniCase::new(lhs).cmp(&UniCase::new(rhs)) { - std::cmp::Ordering::Less => -1, - std::cmp::Ordering::Equal => 0, - std::cmp::Ordering::Greater => 1, - } -} - fn set_log_level(level: ffi::LogLevel) -> Result<(), VerboseError> { libloot::set_log_level(level.try_into()?); Ok(()) @@ -295,8 +286,6 @@ mod ffi { contents: &[MessageContent], language: &str, ) -> OptionalMessageContentRef; - - fn compare_filenames(lhs: &str, rhs: &str) -> i8; } extern "Rust" { @@ -633,6 +622,18 @@ mod ffi { pub fn as_str(&self) -> &str; pub fn boxed_clone(&self) -> Box; + + pub fn eq(&self, other: &Filename) -> bool; + + pub fn ne(&self, other: &Filename) -> bool; + + pub fn lt(&self, other: &Filename) -> bool; + + pub fn le(&self, other: &Filename) -> bool; + + pub fn gt(&self, other: &Filename) -> bool; + + pub fn ge(&self, other: &Filename) -> bool; } extern "Rust" { diff --git a/cpp/src/metadata.rs b/cpp/src/metadata.rs index 7cbf6085..72c11ad5 100644 --- a/cpp/src/metadata.rs +++ b/cpp/src/metadata.rs @@ -446,7 +446,7 @@ impl From> for libloot::metadata::File { } } -#[derive(Clone, Debug)] +#[derive(Clone, Debug, PartialEq, PartialOrd)] #[repr(transparent)] pub struct Filename(libloot::metadata::Filename);