killertux / solder

Library to help you build php extensions using Rust
Other
21 stars 13 forks source link

Not enough unsafe #4

Open wizzwizz4 opened 4 years ago

wizzwizz4 commented 4 years ago

Several APIs inside solder should be marked unsafe, but aren't. From the Nomicon:

The unsafe keyword has two uses: to declare the existence of contracts the compiler can't check, and to declare that a programmer has checked that these contracts have been upheld.

This is important because it makes it a lot harder to reason about Solder than it should be – and, in fact, the code is currently making false promises. It is impossible to use safe code to invoke undefined behaviour, but it is possible to use many parts of Solder to invoke undefined behaviour; ergo, those parts of Solder should not be marked as safe.

Some examples of what should be unsafe (non-exhaustive):

The above two can be fixed either by just marking the relevant functions as unsafe, or by using std::ffi::CStr (hence implementing c_str! using std::ffi::CStr::from_bytes_with_nul) and a safe wrapper around them. Other issues can be fixed in a similar way.

killertux commented 4 years ago

I totally agree. There are more cases that need to be fixed. I will take a look at it.