search

rust-x-bindings / rust-xcb

Rust bindings and wrapper for XCB.
MIT License
165 stars 64 forks source link

Why is XidNew::new() marked unsafe? #197

Open kelnos opened 2 years ago

kelnos commented 2 years ago

My understanding is that unsafe should be used to mark code that requires that rustc disable some of its memory safety checks. But I don't see how passing an invalid XID to XidNew::new() can ever cause memory safety issues. The worst that can happen is that the X server will reply to a request with an error (like BadWindow), which the application will be forced to handle. This doesn't seem meaningfully different to me than having a valid XID, acquired from Connection::generate_id(), that later becomes invalid due to some code destroying the resource (or even the X server doing so, out of your application's control), and then attempting to use it. Either way, there do not seem to be any safety issues.

Am I missing something here?

(I also just noticed #167, and I think the same question applies to that. Passing an invalid or otherwise inappropriate file descriptor obviously will not work very well, but should not cause memory safety issues. If it does, then the underlying code itself has safety issues that should be worked out.)

rtbo commented 2 years ago

You are correct. Unfortunately XidNew::new is a public API, therefore I would need to bump a major version to change it.

kelnos commented 2 years ago

Ah, didn't realize removing unsafe from a trait would be a source-incompatible change, but I guess that makes sense that's the case.