search

tokio-rs / io-uring

The `io_uring` library for Rust
Apache License 2.0
1.19k stars 131 forks source link

lib: make `register_buffers` unsafe #219

Closed fathyb closed 1 year ago

fathyb commented 1 year ago

This breaking change is required as calling register_buffers with incorrect inputs may introduce undefined behaviour.

This PR also changes the method documentation to use fixed buffers instead of user buffers to match the io_uring man pages terminology, related to #217.

FrankReh commented 1 year ago

I don't mind the breaking change and you raise a good point for a crate that wants to be sound. Could you provide a Safety comment that was more in the affirmative of what the caller has to do to maintain the safety contract? Saying it should not be used improperly is more vague than other tokio-maintained unsafe comments.

fathyb commented 1 year ago

I've tweaked the message to match the one on Submission::push, let me know what you think!

FrankReh commented 1 year ago

Yes, looks better to me. @quininer is in a different timezone. Have to wait for their review.

quininer commented 1 year ago

This makes sense. I will release a 0.6-pre first so that I can merge other break changes in the meantime.

quininer commented 1 year ago

I noticed that register_buf_ring is still not marked unsafe and ring_addr is not an address type. maybe we should change it too. cc @FrankReh

FrankReh commented 1 year ago

I noticed that register_buf_ring is still not marked unsafe and ring_addr is not an address type. maybe we should change it too. cc @FrankReh

Yes. I can see the value in making those changes now too.

You think the ring_addr type should be made *const BufRingEntry? You give me a day or two to make a PR?

quininer commented 1 year ago

*const BufRingEntry is fine, please feel free to open PR. :)

FrankReh commented 1 year ago

*const BufRingEntry is fine, please feel free to open PR. :)

Checkout PR 221. I didn't include a type change because as the test code shows, a user of the library is not always really starting with a BufRingEntry array or vector and casting to a u64 is more directly what the io_uring API calls for.

But I'm on the fence. If you want it, I will do that too. I'm just not sure so wanted you to take a second look first.