First ffi project: what can I improve?

code review
1

Hi Everyone,

I have experimented a bit with the cmake crate and want to create my first project which provides Rust bindings for the existing VTK package or exposes some of their information.

Structure

My Project structure looks like this:

├── build.rs
├── Cargo.toml
├── libvtkrs
│   ├── CMakeLists.txt
│   ├── vtkNamedColors.cpp
│   ├── vtkNamedColors.h
│   ├── ...
└── src
    ├── lib.rs
    ├── vtk_named_colors.rs
    ├── ...

I have created an intermediate C++ package with cmake which provides C-style functions that can be linked to the Rust library. These wrapper functions live in separate vtkSomeObject.h and vtkSomeObject.cpp files within the cmake project.
A corresponding Rust module exists (with snake case naming convention) vtk_some_object.rs.

Example

In this example, I am in particular worried about the static cast but I did not find any better way how to pass a pointer referencing the vtkNamedColors object to Rust. Another alterative would be to define the struct vtkNamedColors in the C++ code instead of Rust and then bind to this as well.

// libvtkrs/vtkNamedColors.cpp
#include <vtkNamedColors.h>

extern "C" void *named_colors_new() {
    vtkNamedColors *colors = vtkNamedColors::New();
    return static_cast<void *>(colors);
}

extern "C" void named_colors_delete(void *object_ptr) {
    vtkNamedColors *object;
    static_cast<vtkNamedColors *>(object_ptr)->Delete();
}

// libvtkrs/vtkNamedColors.h
extern "C" void *named_colors_new();
extern "C" void named_colors_delete(void *object_ptr);

// src/vtk_named_colors.rs

use core::ffi::{c_char, c_void};
use std::ffi::CString;

unsafe extern "C" {
    fn named_colors_new() -> *mut c_void;
    fn named_colors_delete(named_colors_ptr: *mut c_void);
}

pub struct vtkNamedColors {
    named_colors_ptr: *mut c_void,
}

pub trait Colors {
    type Color;
    fn GetColor(&self) -> Self::Color;
    fn ResetColors(&mut self);
    fn SetColor(&mut self, name: &str, color: &Self::Color);
    fn GetColorNames(&self) -> String;
    fn GetSynonyms(&self) -> String;
}

impl vtkNamedColors {
    pub fn New() -> Self {
        Self {
            named_colors_ptr: unsafe { named_colors_new() },
        }
    }
}

impl Drop for vtkNamedColors {
    fn drop(&mut self) {
        unsafe { named_colors_delete(self.named_colors_ptr) };
    }
}

Thanks in advance for your help.

2

Your code looks okay in general.

  1. In Rust, building of C/C++ code and extern "C" definitions are usually split into their own crate -sys crate with a links property. That property doesn't do the linking, but tells Cargo that you do. It prevents multiple versions of the Rust package from clobbering their global C symbols.
    It also helps Linux distros to override the build script to replace vendored code with their own.

    Higher-level wrappers can go to another non-sys crate, and can be versioned separately.

  2. In the C wrapper be careful about the names being globally global. Add your own unique prefix to the functions, like vtk_rust_ffi_named_colors_new.

  3. A lot of the boilerplate you write can be semi-automatic. Use the foreign-types crate for memory management of C objects. That crate is a de-facto standard in the Rust ecosystem.

  4. A trait for the interface is usually an overkill, and it's a hassle to use because the trait name needs to be imported first. Use regular inherent methods (impl vtkNamedColors block), unless you really have multiple implementations of the same interface that must be dynamically substituted.
    In Rust anybody can define and implement a trait for your struct, so you don't have to provide one yourself just in case.

  5. In the higher-level wrapper you could try using more Rust-idiomatic names. vtk::NamedColors, color() & set_color, rather than vtkNamedColors and GetColor/SetColor.

  6. GetColor could try to return &str. There's std::slice::from_raw_parts and std::str::from_utf8 for getting &str from pointer + length, and ffi::CStr for getting &str from \0-terminated strings. The &self on the getter will automatically ensure that the object cannot be deleted while the borrowed &str is in use.

3 Likes
3

Thanks for the very detailed and long suggestions! I really appreciate that.

  1. I will look further into this and see how I can restructure the project.
  2. Thanks for the note! I will consider this. Is there any "standardized" way to circumvent this?
  3. I will definitely check out this crate! Thanks
  4. In VTK, there are more object which inherit from a similar base class and thus share this implementation. I thus thought that it would be a good idea to recreate this interface in Rust.
  5. When writing these higher-level wrappers, I was unsure how to do this. On the one hand I want them to follow the convention of the ecosystem but on the other I want to mimic existing functionality since they really are "thin" wrappers around the existing methods.
  6. I tried multiple implementations in this particular method and returning a valid UTF8 String is clearly the better result. I will revisit this.

Overall: Thank you so much for the suggestions.

4

For linking C symbols, adding a custom prefix and hoping nobody else uses that prefix is the standard solution.

Rust and C++ essentially do the same thing for their own namespaced symbols, just with a more obscure mangling scheme instead of a simple prefix. Trying to use Rust's mangled names directly from C would be a major hassle. C++'s mangling can be compiler-specific, so custom plain C names are the best you can do simply and portably.

I don't think you could rely on limited symbol visibility to avoid collisions, since linking needs to be done across C, Rust, and potentially even other languages (Rust crates can be exported as static libraries), and Rust has very little control over linking.

If you have a lot of C++ code to wrap, also check out autocxx:

If you need inheritance, then yeah, traits may be necessary. This is something that gtk-rs needed, so maybe it could be a reference in case you need to solve something more complex. This is unfortunately an impedance mismatch between Rust and OOP languages, so try to use composition and flatten the interface as much as you can get away with.

For the higher-level wrapper, as long as you stick to a straightforward mapping of CamelCase → snake_case, it should be fine.

Rustdoc supports #[doc(alias = "name")] that you can add to make C/C++ names findable in the docs: