search

mathstuf / rust-keyutils

Rust interface to the Linux keyring
BSD 3-Clause "New" or "Revised" License
17 stars 9 forks source link

{Key, Keyring}::new should (probably) not be unsafe #56

Open Nemo157 opened 3 years ago

Nemo157 commented 3 years ago

As far as I can tell inventing an invalid key id will just cause errors on future API calls, there is no way to trigger memory unsafety, so this function should not be marked unsafe. To reduce the chance of users using this incorrectly it could be renamed to new_unchecked to highlight that it's up to the user to ensure they use a valid id.

mathstuf commented 3 years ago

https://doc.rust-lang.org/std/num/struct.NonZeroI32.html#method.new_unchecked is unsafe. I do like it better as a name, but the thing is that there's no way to know if a given ID is valid or not on our side to panic! if it is invalid (not to mention inherent TOCTOU issues).

Nemo157 commented 3 years ago

Hmm, I thought there was some convention for safe _unchecked functions, but looks like I was misremembering. In that case, just remove the unsafe and clarify in the docs that an invalid id will likely lead to future errors?