Using rustified enums in function signatures is (almost?) always wrong · Issue #3051 · rust-lang/rust-bindgen (original) (raw)

So this is an issue that came up from #2908. If you have the following headers:

typedef enum State {Working = 1, Failed = 0} State;

takes_state(enum State state); enum State returns_state();

Runing bindgen --rustified-enum=State will produce this code:

#[repr(u32)] #[derive(Debug, Copy, Clone, Hash, PartialEq, Eq)] pub enum State { Working = 1, Failed = 0, } extern "C" { pub fn takes_state(state: State); } extern "C" { pub fn returns_state() -> State; }

Sadly, using takes_state or returns_state can cause undefined behavior. This is documented behavior but it could easily be improved by adding appropriate conversion methods for State (which is part of the purpose of #2908). Additionally, bindgen could expose the underlying type of the C enum and use this type instead of State.

Essentially, I would expect bindgen to generate:

#[repr(u32)] #[derive(Debug, Copy, Clone, Hash, PartialEq, Eq)] pub enum State { Working = 1, Failed = 0, }

pub type State_ctype = ::std::os::raw::c_uint;

// This can be optionally generated (#2908) impl State { const unsafe fn from_ctype_unchecked(v: State_ctype) -> Self { std::mem::transmute(v) } }

// This can be optionally generated impl From for State_ctype { fn from(value: State) -> Self { value as Self } }

// This can be optionally generated (#2908) impl TryFrom for State { type Err = InvalidState;

fn try_from(value: State_ctype) -> Result<Self, Self::Err> {
    match value {
        0 => Ok(Self::Working),
        1 => Ok(Self::Failed),
         _ => Err(InvalidState(value)),
    }
}

}

// Here we no longer use State but State_ctype. extern "C" { pub fn takes_state(state: State_ctype); } extern "C" { pub fn returns_state() -> State_ctype; }

The main advantages of this approach is its flexibility as the user can explicitly opt-in for the conversion methods according to the safety guarantees. If your C library is kind enough to never produce invalid values for an enum, then you can codify that behavior

// Safety: returns_state is always safe to call and it is guaranteed to produce a valid State. unsafe { State::from_ctype_unchecked(returns_state())) }

However, if you're unsure, you can deal with this accordingly and expose a Rust function that acknowledges this:

fn returns_state() -> Result<State, <State as TryFrom>::Err> { // Safety: returns_state is always safe to call unsafe { bindings::returns_state() }.try_into() }

The additional improvement over the current behavior is whentakes_state and returns_state are used in conjunction:

unsafe { takes_state(returns_state()) }

With the current behavior returns_state can create an invalid value for State, which is UB. Then this is passed to takes_state and "hopefully" all works out. By using the State_ctype all the unsafety would only happen on the C side.

Cc @emilio @jbaublitz @ojeda