On Tue, Oct 22, 2024 at 9:12 AM Junjie Mao <junjie....@hotmail.com> wrote: > My understanding is that: > > 1. init_fn() is called by QEMU QOM subsystem with certain timing > (e.g., called after main()) and concurrency (e.g., all init_fn() > are called sequentially) preconditions. > > 2. The caller of module_init! is responsible for justifying the safety > of their $body under the preconditions established in #1. > > "unsafe fn" in Rust is designed to mark functions with safety > preconditions so that the compiler can raise an error if the caller > forgets that it has those preconditions to uphold [1]. > > Under that interpretation, a safe "fn init_fn()" reads that init_fn() is > safe to call without the preconditions mentioned in #1. That is rarely > the case to me. > > Using "unsafe" to mark the responsibility of device backends in #2 looks > like repurposing the keyword. That may make more sense in this specific > context because: > > 1. the callers of init_fn() are in C, so Rust compiler cannot really > check them, > > 2. the caller will by design upholds the safety preconditions > regardless of whether a specific callback needs it or not, and
Or at least will try. > 3. without unsafe_op_in_unsafe_fn an unsafe fn hides unsafe operation > in its body from the compiler. 4. In this specific case, init_fn is only used from ctor_fn and even there only as a function pointer; it is not visible from outside. So the "unsafe" marker's only purpose is documentation (and in fact, as you point out in (3), it would even be harmful without unsafe_op_in_unsafe_fn). > If that's what we prefer, I would suggest add the considerations above > to the code as comments, so that future readers are not confused by the > usage of "unsafe" here. Yes, I agree that since we're talking about a macro (not a function which can itself be annotated with "unsafe") that lies at the C<->Rust boundary, this is mostly a documentation issue. In general the documentation of qemu-api and qemu-api-macros leaves something to be desired. This is okay, but it is also the kind of technical debt that we need to look at as soon as we can actually get the thing to compile. :) I'll add it shortly to https://wiki.qemu.org/Features/Rust. Paolo
