Looking for feedback on API bindings for a C library

code review
1

I already received lots of useful feedback on other, smaller scale, problems and questions I had about the Rust language, and I'm grateful for that. Today I'm looking for a bit of code review :slight_smile:

I have recently published Rust bindings for the bddisasm C library. The C library is a x86/x86_64 instruction decoder. It is fairly small, with a simple API, but outputs the results in a pretty complex structure with quite a lot of information provided, ranging from how to check if a CPU supports an instruction to in-depth information about the instruction operands. I attempted to wrap all of this into a idiomatic Rust API, following the API guidelines as close as I could.

There are two crates involved:

  • bddisasm-sys - this includes the FFI bindings (generated with bindgen) and nothing else
  • bddisasm - an attempt at a higher level API

The code for both is available here. I appreciate any kind of feedback, ranging from API design to directory structure, documentation, etc. I'm very much a beginner when it comes to Rust, and apart from some small projects and tools only I (and one or two other people) use I don't have much experience with the language.

I can't link to the docs yet because I'm still in the build queue on docs.rs at the moment.

At the moment I know that the documentation should be more in-depth, with more examples and I feel a bit uneasy about how I dumped all the source files in src.

I generate the bindings at build time so you'll need libclang in order to build it. I'm not sure how often this is encountered with other similar crates, but I know that this is considered an issue by some people.

1 Like
2

I've just been clicking through the docs, and I don't know anything about x86 instruction encoding, but this looks pretty good to me—nice work!

A handy resource for this stuff is @kornel's article about sys crates, if you haven't read it already.

You don't need to write extern crate bddisasm_sys as ffi in each file, just put it in lib.rs and ffi:: paths will be available in the rest of the crate. You might consider making it pub too, so it's easy for consumers of the crate to access (under bddisasm::ffi::) a version of bddisasm-sys that's synchronized with their version of bddisasm.

I think a flat module structure is fine, actually. If you feel like a different hierarchy would better reflect how you think about the crate's organization or make it easier to use then go for it, but I don't see a problem with how it is currently.

3

That was my starting point. Really helpful. Without it I would have bundled the FFI bindings and API bindings in the same crate.

Thanks! Will do that.

This sounds really helpful. I was considering giving access to the inner struct as well.

I looked around at other popular crates and most of them have a mix: a few files directly in src and a few directories. I'm starting to think that it's fine, giving the small file count.

Thank you :slight_smile:

1 Like
4

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.