On Mon, Oct 21, 2024 at 05:47:05PM +0100, Peter Maydell wrote:
> Date: Mon, 21 Oct 2024 17:47:05 +0100
> From: Peter Maydell <peter.mayd...@linaro.org>
> Subject: Re: [Question] What is the definition of “private” fields in
> QOM?
>
> 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).
Thank you for taking me through this history, I see the intent of
designing the embedded device structure in this way!
> 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.
Very cool! May I ask a few more questions? :-) The feedback on this
series looks very positive, and it seems you were almost close to merge
it at the time. What ultimately led you to decide against it? If we
revisit the issue of Rust's private/pub visibility, I'm curious if we
would face the same dilemma again.
Regards,
Zhao
