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.

4 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:

1 Like
5

Hi @kornel and thanks again for the useful tips. Over the past days I have continued to work on the crate vtk-rs. Right now I am using a combination of your suggestions. Using cxx was a game-changer for me although I can not utilize it directly, but need to generate the appropriate header files with the CLI tool cxxbridge-cmd.

At this point in time, almost nothing of the original API is exposed. This is on purpose since I am refining how to best approach many of the problems which I am facing.

Another problem which I have not solved is how to correctly locate system libraries. Right now I am simply guessing for various operating systems and version numbers. I have found some tools which might retrieve such informations but they rely on external package managers.
Right now, I plan to approach it as follows:

  1. Try to automate discovery based on heuristics which work for the major distributions (ubuntu, macos, archlinux)
  2. Have fallback mechanicsm to explicitly specify how to localize system libs.

For step 2, I wanted to use environment variables such as VTK_DIR or VTK_VERSION. I saw that there are a few ways to embed meta-data into the Cargo.toml file but since installation directories might differ between users, I thought that this is not appropriate.

6

System libraries will unfortunately vary between systems. These problems with library discovery, versioning, compatibility, configuration, etc. are usually handled by dedicated *-sys crates (as mentioned in my previous post).

Cargo won't help here, other than with the basics of running build.rs and running your linker directives. There's no silver bullet here, because you're dealing with many different quirks of many different systems, sometimes having conflicting requirements (e.g. Debian packaging forbids bundling of dependencies, distribution on Windows requires bundling).

If you want to make that easier for users, it goes beyond Cargo into packaging and distribution. There's cargo-dist, but you may need to just manually prepare a package or installation instructions for each OS you support.

7

Note that cargo-dist is apparently unmaintained (they ran out of funding). Source: PSA: cargo-dist is dead : rust

8

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.