On Mon, 21 Oct 2024 at 16:25, Zhao Liu <zhao1....@intel.com> wrote:
> My initial confusion stemmed from seeing the private comment and
> noticing that there are many direct accesses to parent_obj/parent_class
> in QEMU (which I could list in my reply to Daniel). Now I understand
> that these examples are only valid within the class/object
> implementation or in QOM code.
>
> For instance, an example in KVM is a misuse:
>
> accel/kvm/kvm-all.c:4285:
> cpu->parent_obj.canonical_path,
> accel/kvm/kvm-all.c:4377: if
> (!apply_str_list_filter(cpu->parent_obj.canonical_path, targets)) {
>
>
> At the same time, I’ve been thinking that the current C implementation
> seems to have no way to prevent such misuse.
Mmm. We rely on code review to catch major misuses (and let
some minor misuses slide because we don't care enough to put
in the effort to provide a proper API to access the information
or because we don't want the performance overhead of a QOM cast).
In a previous iteration of QEMU's design the device's state
struct was purely private to the implementation source file,
and code that used the device always did so via a pointer.
But at some point we decided we wanted to allow users to
embed the device struct inside their own device struct, which
needs them to have access to the struct definition (though
strictly they only need to know the size and alignment
requirements of it).
I did a decade ago write a proof-of-concept for marking
fields in the C struct as "private" such that you could get
a compile error for touching them:
https://lists.gnu.org/archive/html/qemu-devel/2014-05/msg01846.html
which (mis?)uses GCC's __attribute__((deprecated("reason")))
to arrange that touching the struct field from outside the
implementation is a compile error. But we never took up the idea.
> So for Rust's class/state,
> should parent_class/parent_obj also be defined as private (for example,
> by removing the pub keyword from PL011State in rust/hw/char/pl011/src/
> device.rs)?
>
> However, through our discussion, I realized that in QOM, "private" does
> not only refer to parent_obj/parent_class, but all fields belong to
> this category. If everything is strictly defined as private in Rust, it
> seems impractical…
For Rust we get to make a fresh start on these things. If
we do mark all these fields not public, what would break?
I do think that some of these fields really are internal
implementation details -- only the PL011 UART itself
should be directly accessing PL011State::read_fifo, for example.
thanks
-- PMM
