Skip to content

Commit

Permalink
Move utils bridged methods into their own ffi module
Browse files Browse the repository at this point in the history
Discovered that we can use `Pin<&mut ...>` to allow mutation of Rust
types defined in another cxx bridge module, based on:
- https://cxx.rs/extern-c++.html#opaque-c-types
  > Unlike extern Rust types and shared types, an extern C++ type is not
  > permitted to be passed by plain mutable reference &mut MyType across the
  > FFI bridge. For mutation support, the bridge is required to use Pin<&mut
  > MyType>. This is to safeguard against things like mem::swap-ing the
  > contents of two mutable references, given that Rust doesn't have
  > information about the size of the underlying object and couldn't invoke
  > an appropriate C++ move constructor anyway.
- dtolnay/cxx#942
  • Loading branch information
dennisschagt committed Apr 4, 2024
1 parent 73db155 commit 4b8034c
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 40 deletions.
2 changes: 1 addition & 1 deletion include/filepath.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class Filepath {
{
}

Filepath(rust::Box<filepath::bridged::PathBuf> rs_object)
Filepath(rust::Box<filepath::bridged::PathBuf>&& rs_object)
: rs_object(std::move(rs_object))
{
}
Expand Down
27 changes: 0 additions & 27 deletions rust/libnewsboat-ffi/src/filepath.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,6 @@ mod bridged {
fn set_extension(filepath: &mut PathBuf, extension: Vec<u8>) -> bool;
fn starts_with(filepath: &PathBuf, str: Vec<u8>) -> bool;
fn file_name(filepath: &PathBuf) -> Vec<u8>;

// These functions are actually in utils.rs, but I couldn't find a way to return
// `Box<PathBuf>` from libnewsboat-ffi/src/utils.rs, so I moved the bindings here
fn get_default_browser() -> Box<PathBuf>;
fn resolve_tilde(path: &PathBuf) -> Box<PathBuf>;
fn resolve_relative(reference: &PathBuf, path: &PathBuf) -> Box<PathBuf>;
fn getcwd() -> Box<PathBuf>;
}
}

Expand Down Expand Up @@ -67,26 +60,6 @@ fn clone(filepath: &PathBuf) -> Box<PathBuf> {
Box::new(PathBuf(filepath.0.clone()))
}

fn get_default_browser() -> Box<PathBuf> {
Box::new(PathBuf(libnewsboat::utils::get_default_browser()))
}

fn resolve_tilde(path: &PathBuf) -> Box<PathBuf> {
Box::new(PathBuf(libnewsboat::utils::resolve_tilde(path.0.clone())))
}

fn resolve_relative(reference: &PathBuf, path: &PathBuf) -> Box<PathBuf> {
Box::new(PathBuf(libnewsboat::utils::resolve_relative(
&reference.0,
&path.0,
)))
}

fn getcwd() -> Box<PathBuf> {
let result = libnewsboat::utils::getcwd().unwrap_or_else(|_| std::path::PathBuf::new());
Box::new(PathBuf(result))
}

fn is_absolute(filepath: &PathBuf) -> bool {
filepath.0.is_absolute()
}
Expand Down
21 changes: 21 additions & 0 deletions rust/libnewsboat-ffi/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use crate::filepath::PathBuf;
use libc::{c_char, c_ulong};
use libnewsboat::utils::{self, *};
use std::ffi::{CStr, CString};
use std::pin::Pin;

#[cxx::bridge(namespace = "newsboat::utils")]
mod ffi {
Expand Down Expand Up @@ -79,6 +80,10 @@ mod bridged {
fn tokenize_quoted(line: &str, delimiters: &str) -> Vec<String>;
fn is_valid_podcast_type(mimetype: &str) -> bool;

fn get_default_browser(mut path: Pin<&mut PathBuf>);
fn resolve_tilde(path: &PathBuf, mut output: Pin<&mut PathBuf>);
fn resolve_relative(reference: &PathBuf, path: &PathBuf, mut output: Pin<&mut PathBuf>);
fn getcwd(mut path: Pin<&mut PathBuf>);
fn mkdir_parents(path: &PathBuf, mode: u32) -> isize;

fn unescape_url(url: String, success: &mut bool) -> String;
Expand Down Expand Up @@ -189,6 +194,22 @@ fn extract_filter(line: &str) -> ffi::FilterUrlParts {
}
}

fn get_default_browser(mut path: Pin<&mut PathBuf>) {
path.0 = utils::get_default_browser();
}

fn resolve_tilde(path: &PathBuf, mut output: Pin<&mut PathBuf>) {
output.0 = utils::resolve_tilde(path.0.clone());
}

fn resolve_relative(reference: &PathBuf, path: &PathBuf, mut output: Pin<&mut PathBuf>) {
output.0 = utils::resolve_relative(&reference.0, &path.0);
}

fn getcwd(mut path: Pin<&mut PathBuf>) {
path.0 = utils::getcwd().unwrap_or_else(|_| std::path::PathBuf::new());
}

fn mkdir_parents(path: &PathBuf, mode: u32) -> isize {
match utils::mkdir_parents(&path.0, mode) {
Ok(_) => 0,
Expand Down
24 changes: 12 additions & 12 deletions src/utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -340,17 +340,17 @@ std::string utils::run_program(const char* argv[], const std::string& input)

Filepath utils::resolve_tilde(const Filepath& path)
{
// The function is in utils.rs, but its binding is in filepath.rs because
// I can't make it return `Box<PathBuf>` any other way
return filepath::bridged::resolve_tilde(path);
auto output = filepath::bridged::create_empty();
utils::bridged::resolve_tilde(path, *output);
return output;
}

Filepath utils::resolve_relative(const Filepath& reference,
const Filepath& fname)
{
// The function is in utils.rs, but its binding is in filepath.rs because
// I can't make it return `Box<PathBuf>` any other way
return filepath::bridged::resolve_relative(reference, fname);
auto output = filepath::bridged::create_empty();
utils::bridged::resolve_relative(reference, fname, *output);
return output;
}

std::string utils::replace_all(std::string str,
Expand Down Expand Up @@ -719,9 +719,9 @@ nonstd::optional<std::uint8_t> utils::run_non_interactively(

Filepath utils::getcwd()
{
// The function is in utils.rs, but its binding is in filepath.rs because
// I can't make it return `Box<PathBuf>` any other way
return filepath::bridged::getcwd();
auto path = filepath::bridged::create_empty();
utils::bridged::getcwd(*path);
return path;
}

nonstd::expected<std::vector<std::string>, utils::ReadTextFileError> utils::read_text_file(
Expand Down Expand Up @@ -842,9 +842,9 @@ void utils::initialize_ssl_implementation(void)

Filepath utils::get_default_browser()
{
// The function is in utils.rs, but its binding is in filepath.rs because
// I can't make it return `Box<PathBuf>` any other way
return filepath::bridged::get_default_browser();
auto path = filepath::bridged::create_empty();
utils::bridged::get_default_browser(*path);
return path;
}

std::string utils::md5hash(const std::string& input)
Expand Down

0 comments on commit 4b8034c

Please sign in to comment.