Sorry, I didn’t see this: > On 29 Oct 2024, at 07:18, Alice Ryhl <aliceryhl@xxxxxxxxxx> wrote: > > What you're doing now is a bit awkward to use. You have to make sure > that it never escapes the struct it's created for, so e.g. you can't > give out a mutable reference as the user could otherwise `mem::swap` > it with another Io. Similarly, the Io can never be in a public field. > Your safety comment on Io::new really needs to say something like > "while this struct exists, the `addr` must be a valid I/O region", > since I assume such regions are not valid forever? Similarly if we Io is meant to be a private member within a wrapper type that actually acquires the underlying I/O region, like `pci::Bar` or `Platform::IoMem`. Doesn’t that fix the above? > look at [1], the I/O region actually gets unmapped *before* the Io is > destroyed since IoMem::drop runs before the fields of IoMem are > destroyed, so you really need something like "until the last use of > this Io" and not "until this Io is destroyed" in the safety comment. > > If you compare similar cases in Rust, then they also do what I > suggested. For example, Vec<T> holds a raw pointer, and it uses unsafe > to assert that it's valid on each use of the raw pointer - it does not > create e.g. an `&'static mut [T]` to hold in a field of the Vec<T>. > Having an IoRaw<S> and an Io<'a, S> corresponds to what Vec<T> does > with IoRaw being like NonNull<T> and Io<'a, S> being like &'a T. > > [1]: https://lore.kernel.org/all/20241024-topic-panthor-rs-platform_io_support-v1-1-3d1addd96e30@xxxxxxxxxxxxx/ What I was trying to say in my last message is that the wrapper type, i.e.: IoMem and so on, should not have a lifetime parameter, but IIUC this is not what you’re suggesting here, right? — Daniel
