Michael-F-Bryan / rust-ffi-guide

A guide for doing FFI using Rust
https://michael-f-bryan.github.io/rust-ffi-guide/
Creative Commons Zero v1.0 Universal
282 stars 18 forks source link

Started a rewrite #65

Open Michael-F-Bryan opened 6 years ago

Michael-F-Bryan commented 6 years ago

(Rendered) (fixes #64)

richard-uk1 commented 6 years ago

We also need to update magic-sys's Cargo.toml file to tell cargo about our build script and let it know we link to libmagic (this prevents accidentally linking to two versions of magic-sys because then bad things can happen).

I think if you call your build script build.rs then it 'just works' now.

One could argue that explicitness is better though

richard-uk1 commented 6 years ago
// creation failed. Release MAGIC_CREATED (in drop) and
// return an error
drop(magic);

One of the things I always struggle with is exactly what you need to clean up when, especially when you're in a failure state. I've seen lots of C libs that use goto in this situation, because the default control flow structures just don't cut it. Here is an example. The guide could talk about this/mention that you may need to look in the C source code to know exactly what you have to free when.

Michael-F-Bryan commented 6 years ago

One of the things I always struggle with is exactly what you need to clean up when, especially when you're in a failure state.

As a general rule, you'll want to release any resources you acquired during the object's lifetime. In this case the only thing that was acquired was the MAGIC_CREATED "lock", so that's all we need to release.

I've seen lots of C libs that use goto in this situation, because the default control flow structures just don't cut it.

This is true and actually one of the few times where I'd promote the use of goto (assuming there are no better constructs available). A try-catch block is essentially the same as using goto cleanup in that you stop what you're doing and jump straight to the cleanup code. In C++ and Rust we typically use destructors for the same purpose.

EDIT: Ooh, I just found a bug in my code! We shouldn't be using drop() there because it'll also prematurely free libmagic's internal state for the already active instance of Magic.