[Qemu-devel] a QOM Coding Conventions question

[Qemu-devel] a QOM Coding Conventions question

[Antony Pavlov]

Hi, Andreas!

Here is a quote from http://wiki.qemu.org/QOMConventions
    
>     a FooClass structure definition containing at least the parent class field: 
>
>     typedef struct {
>         /*< private >*/
>         MyParentClass parent_class;
>         /*< public >*/
>
>         [any fields you need]
>     } FooClass;

What do the "< private >" and "< public >" comments exactly mean here?

-- 
Best regards,
  Antony Pavlov

Re: [Qemu-devel] a QOM Coding Conventions question

[Peter Crosthwaite]

On Sat, Dec 14, 2013 at 6:26 AM, Antony Pavlov <antonynpavlov@gmail.com> wrote:
> Hi, Andreas!
>
> Here is a quote from http://wiki.qemu.org/QOMConventions
>
>>     a FooClass structure definition containing at least the parent class field:
>>
>>     typedef struct {
>>         /*< private >*/
>>         MyParentClass parent_class;
>>         /*< public >*/
>>
>>         [any fields you need]
>>     } FooClass;
>
> What do the "< private >" and "< public >" comments exactly mean here?

Private means inaccessible to everybody, including the implementation
of class being instantiated. No one should ever dereference a private
variable, they should be managed by QOM indirectly via casts if
needed. Public means that at least someone can access it. Note that
public does not declare a free-for-all. QOM class variables may be
"public" in the sense that the class implementation may access them.
Container devices however still can not, and they are private from
that point of view.

For example, a timer peripheral may have a "public" ptimer, in the
sense that the timer class derefences and modifies the timer for its
implmentation. An embedding SoC device however can NOT use this
despite being public, its private to the timer implementation. So in
short:

< /* private */ > - owned by the QOM framwork - do no dereference ever
from anywhere.
< /* public */> - owned by the class implementation - do not
dereference from containers.

There is also automated documentation generation using this.

Regards,
Peter

>
> --
> Best regards,
>   Antony Pavlov
>

Re: [Qemu-devel] a QOM Coding Conventions question

[Peter Maydell]

On 14 December 2013 23:06, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> On Sat, Dec 14, 2013 at 6:26 AM, Antony Pavlov <antonynpavlov@gmail.com> wrote:
>> What do the "< private >" and "< public >" comments exactly mean here?
>
> Private means inaccessible to everybody, including the implementation
> of class being instantiated. No one should ever dereference a private
> variable, they should be managed by QOM indirectly via casts if
> needed. Public means that at least someone can access it. Note that
> public does not declare a free-for-all. QOM class variables may be
> "public" in the sense that the class implementation may access them.
> Container devices however still can not, and they are private from
> that point of view.

I'm not sure I find this a terribly convincing explanation. Last time
I asked this IIRC the answer was just "they need to be there for the
output of the documentation generator".

The usual definition of 'public' vs 'private' is "what can a user of the
object rather than the implementor safely access?". That would be
actually interesting to document IMHO. (I should resurrect my patch
for marking those fields up with __private so we enforce it...)

thanks
-- PMM

Re: [Qemu-devel] a QOM Coding Conventions question

[Peter Crosthwaite]

On Sun, Dec 15, 2013 at 9:42 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 14 December 2013 23:06, Peter Crosthwaite
> <peter.crosthwaite@xilinx.com> wrote:
>> On Sat, Dec 14, 2013 at 6:26 AM, Antony Pavlov <antonynpavlov@gmail.com> wrote:
>>> What do the "< private >" and "< public >" comments exactly mean here?
>>
>> Private means inaccessible to everybody, including the implementation
>> of class being instantiated. No one should ever dereference a private
>> variable, they should be managed by QOM indirectly via casts if
>> needed. Public means that at least someone can access it. Note that
>> public does not declare a free-for-all. QOM class variables may be
>> "public" in the sense that the class implementation may access them.
>> Container devices however still can not, and they are private from
>> that point of view.
>
> I'm not sure I find this a terribly convincing explanation. Last time
> I asked this IIRC the answer was just "they need to be there for the
> output of the documentation generator".
>
> The usual definition of 'public' vs 'private' is "what can a user of the
> object rather than the implementor safely access?". That would be
> actually interesting to document IMHO. (I should resurrect my patch
> for marking those fields up with __private so we enforce it...)
>

Yes, that means we really have three levels of access, but only tags
for two. Hence the confusion over public.

Regards,
Peter

> thanks
> -- PMM
>

Re: [Qemu-devel] a QOM Coding Conventions question

[Pantelis Koukousoulas]

[-- Attachment #1: Type: text/plain, Size: 1753 bytes --]

Why not add "protected" then for the fields that are only
meant to be accessible by the implementation but not
public ?


On Sun, Dec 15, 2013 at 1:51 AM, Peter Crosthwaite <
peter.crosthwaite@xilinx.com> wrote:

> On Sun, Dec 15, 2013 at 9:42 AM, Peter Maydell <peter.maydell@linaro.org>
> wrote:
> > On 14 December 2013 23:06, Peter Crosthwaite
> > <peter.crosthwaite@xilinx.com> wrote:
> >> On Sat, Dec 14, 2013 at 6:26 AM, Antony Pavlov <antonynpavlov@gmail.com>
> wrote:
> >>> What do the "< private >" and "< public >" comments exactly mean here?
> >>
> >> Private means inaccessible to everybody, including the implementation
> >> of class being instantiated. No one should ever dereference a private
> >> variable, they should be managed by QOM indirectly via casts if
> >> needed. Public means that at least someone can access it. Note that
> >> public does not declare a free-for-all. QOM class variables may be
> >> "public" in the sense that the class implementation may access them.
> >> Container devices however still can not, and they are private from
> >> that point of view.
> >
> > I'm not sure I find this a terribly convincing explanation. Last time
> > I asked this IIRC the answer was just "they need to be there for the
> > output of the documentation generator".
> >
> > The usual definition of 'public' vs 'private' is "what can a user of the
> > object rather than the implementor safely access?". That would be
> > actually interesting to document IMHO. (I should resurrect my patch
> > for marking those fields up with __private so we enforce it...)
> >
>
> Yes, that means we really have three levels of access, but only tags
> for two. Hence the confusion over public.
>
> Regards,
> Peter
>
> > thanks
> > -- PMM
> >
>
>

[-- Attachment #2: Type: text/html, Size: 2588 bytes --]

Re: [Qemu-devel] a QOM Coding Conventions question

[Peter Crosthwaite]

On Sun, Dec 15, 2013 at 7:02 PM, Pantelis  Koukousoulas
<pktoss@gmail.com> wrote:
> Why not add "protected" then for the fields that are only
> meant to be accessible by the implementation but not
> public ?
>

Ok by me if the documentation generation can be cleanly and easily
taught to interpret this correctly. I don't know much about the docs
TBH.

Regards,
Peter

>
> On Sun, Dec 15, 2013 at 1:51 AM, Peter Crosthwaite
> <peter.crosthwaite@xilinx.com> wrote:
>>
>> On Sun, Dec 15, 2013 at 9:42 AM, Peter Maydell <peter.maydell@linaro.org>
>> wrote:
>> > On 14 December 2013 23:06, Peter Crosthwaite
>> > <peter.crosthwaite@xilinx.com> wrote:
>> >> On Sat, Dec 14, 2013 at 6:26 AM, Antony Pavlov
>> >> <antonynpavlov@gmail.com> wrote:
>> >>> What do the "< private >" and "< public >" comments exactly mean here?
>> >>
>> >> Private means inaccessible to everybody, including the implementation
>> >> of class being instantiated. No one should ever dereference a private
>> >> variable, they should be managed by QOM indirectly via casts if
>> >> needed. Public means that at least someone can access it. Note that
>> >> public does not declare a free-for-all. QOM class variables may be
>> >> "public" in the sense that the class implementation may access them.
>> >> Container devices however still can not, and they are private from
>> >> that point of view.
>> >
>> > I'm not sure I find this a terribly convincing explanation. Last time
>> > I asked this IIRC the answer was just "they need to be there for the
>> > output of the documentation generator".
>> >
>> > The usual definition of 'public' vs 'private' is "what can a user of the
>> > object rather than the implementor safely access?". That would be
>> > actually interesting to document IMHO. (I should resurrect my patch
>> > for marking those fields up with __private so we enforce it...)
>> >
>>
>> Yes, that means we really have three levels of access, but only tags
>> for two. Hence the confusion over public.
>>
>> Regards,
>> Peter
>>
>> > thanks
>> > -- PMM
>> >
>>
>

Re: [Qemu-devel] a QOM Coding Conventions question

[Andreas Färber]

Hi guys,

Am 16.12.2013 13:48, schrieb Peter Crosthwaite:
> On Sun, Dec 15, 2013 at 7:02 PM, Pantelis  Koukousoulas
> <pktoss@gmail.com> wrote:
>> Why not add "protected" then for the fields that are only
>> meant to be accessible by the implementation but not
>> public ?
>>
> 
> Ok by me if the documentation generation can be cleanly and easily
> taught to interpret this correctly. I don't know much about the docs
> TBH.

/*< private >*/ and /*< public >*/ are the only two visibility tags
supported by gtk-doc [1] (private=off and public=on, basically), so
unless someone sets out to write better tools, that won't work as intended.
And we do want to document what the meaning of certain fields is, so
using private everywhere is counterproductive.

The main reason why the gtk-doc patches [2] keep catching dust is that
the gtk-doc scanner tool makes some assumptions about coding style that
don't hold true for QEMU and thus required a slightly patched version of
gtk-doc (patches posted by Anthony Liguori).
So someone contributing a working gtk-doc compatible scanner (or getting
upstream gtk-doc to accept the necessary changes) might actually be
appreciated - in which case we could restart the discussion of mark-up
extensions.

Until then you can use human-readable comments to indicate that
something is not supposed to be touched (which I personally consider the
historical default though).

That said, the intent in the QOM usage that I adopted from Anthony L. is
to hide the implementation-specific parent field, just like I have been
fighting to get it out of DO_UPCAST() and related usage patterns.
The standardized name parent_obj serves to allow for grep'ing and
together with the requested spacing after the parent_obj field serves to
aid in visually spotting which structs are related to QOM. Apart from
the parent field, we have encouraged to group internal data fields such
as MemoryRegions vs. public child<> and link<> properties.

HTH,
Andreas

[1]
https://developer.gnome.org/gtk-doc-manual/1.18/documenting_symbols.html.en
[2] http://repo.or.cz/w/qemu/afaerber.git/shortlog/refs/heads/gtk-doc

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] a QOM Coding Conventions question

[Antony Pavlov]

On Sun, 15 Dec 2013 09:06:30 +1000
Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:

> On Sat, Dec 14, 2013 at 6:26 AM, Antony Pavlov <antonynpavlov@gmail.com> wrote:
> > Hi, Andreas!
> >
> > Here is a quote from http://wiki.qemu.org/QOMConventions
> >
> >>     a FooClass structure definition containing at least the parent class field:
> >>
> >>     typedef struct {
> >>         /*< private >*/
> >>         MyParentClass parent_class;
> >>         /*< public >*/
> >>
> >>         [any fields you need]
> >>     } FooClass;
> >
> > What do the "< private >" and "< public >" comments exactly mean here?
> 
> Private means inaccessible to everybody, including the implementation
> of class being instantiated. No one should ever dereference a private
> variable, they should be managed by QOM indirectly via casts if
> needed. Public means that at least someone can access it. Note that
> public does not declare a free-for-all. QOM class variables may be
> "public" in the sense that the class implementation may access them.
> Container devices however still can not, and they are private from
> that point of view.
> 
> For example, a timer peripheral may have a "public" ptimer, in the
> sense that the timer class derefences and modifies the timer for its
> implmentation. An embedding SoC device however can NOT use this
> despite being public, its private to the timer implementation. So in
> short:
> 
> < /* private */ > - owned by the QOM framwork - do no dereference ever
> from anywhere.
> < /* public */> - owned by the class implementation - do not
> dereference from containers.
> 
> There is also automated documentation generation using this.
> 

It's a exhaustive comment! Thanks! 
Can we add a comment like this to http://wiki.qemu.org/QOMConventions?

IMHO this public/private QOM convention is a bit confusing. I can remember
about "public/private/protected" in C++/Java. Here is a quote
from http://en.wikipedia.org/?title=C%2B%2B
 
  A public member of the class is accessible to any function.
  A private member is accessible only to functions that are
  members of that class and to functions and classes explicitly
  granted access permission by the class ("friends").

So if you use FooClass' point of view (C++/Java habit) then
 "private" and "protect" confuse you.

Can we use more detailed comments? E.g.

     typedef struct {
         /*< QOM service fields >*/
         MyParentClass parent_class;
         /*< class private fields >*/

         [any fields you need]
     } FooClass;

-- 
Best regards,
  Antony Pavlov

Re: [Qemu-devel] a QOM Coding Conventions question

[Andreas Färber]

Am 15.12.2013 12:17, schrieb Antony Pavlov:
> Can we use more detailed comments? E.g.
> 
>      typedef struct {
>          /*< QOM service fields >*/
>          MyParentClass parent_class;
>          /*< class private fields >*/
> 
>          [any fields you need]
>      } FooClass;

Since those comments wouldn't be supported by gtk-doc, the short answer
is we can't.

The longer answer, as explained in the other reply, is that it's a
question of who contributes and more importantly maintains long-term the
tools to process these and all the /** ... */ comments.

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg