| Message ID | 20230420155723.1711048-9-alex.bennee@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | docs: various (style, punctuation and typo fixes) | expand |
On 20/04/2023 16:57, Alex Bennée wrote: > We have a more complete document on QOM but we should at least mention > the style requirements in the style guide. > > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> > --- > docs/devel/qom.rst | 2 ++ > docs/devel/style.rst | 29 +++++++++++++++++++++++++++++ > 2 files changed, 31 insertions(+) > > diff --git a/docs/devel/qom.rst b/docs/devel/qom.rst > index 3e34b07c98..c9237950d0 100644 > --- a/docs/devel/qom.rst > +++ b/docs/devel/qom.rst > @@ -1,3 +1,5 @@ > +.. _qom: > + > =========================== > The QEMU Object Model (QOM) > =========================== > diff --git a/docs/devel/style.rst b/docs/devel/style.rst > index 5bc6f2f095..0bd01f3fca 100644 > --- a/docs/devel/style.rst > +++ b/docs/devel/style.rst > @@ -628,6 +628,35 @@ are still some caveats to beware of > QEMU Specific Idioms > ******************** > > +QEMU Object Model Declarations > +============================== > + > +The QEMU Object Model (QOM) provides a framework for handling objects > +in the base C language. The first declaration of a storage or class > +structure should always be the parent and leave a visual space between > +that declaration and the new code. > + > +.. code-block:: c > + > + typedef struct MyDeviceState { > + DeviceState parent_obj; > + > + /* Properties */ > + int prop_a; > + char *prob_b; > + /* Other stuff */ > + int internal_state; > + } MyDeviceState; > + > + typedef struct MyDeviceClass { > + ObjectClass parent_class; This one should be DeviceClass in this particular example. > + void (*new_fn1)(void); > + bool (*new_fn2)(CPUState *); > + } MyDeviceClass; > + > +See :ref:`qom` for more details. A couple of points: 1) It is probably worth removing the typedefs given that they are handled by the various QOM macros 2) There should be mention of the fixed names "parent_obj" and "parent_class" for the first declaration. How about something like this: QEMU Object Model Declarations ============================== The QEMU Object Model (QOM) provides a framework for handling objects in the base C language. The first declaration of a storage or class structure should always be the parent and leave a visual space between that declaration and the new code. For a storage structure the first declaration should always be called "parent_obj" and for a class structure the first member should always be called "parent_class" as below: .. code-block:: c struct MyDeviceState { DeviceState parent_obj; /* Properties */ int prop_a; char *prob_b; /* Other stuff */ int internal_state; }; struct MyDeviceClass { DeviceClass parent_class; void (*new_fn1)(void); bool (*new_fn2)(CPUState *); }; Note that there is no need to provide typedefs for QOM structures since these are generated automatically by the QOM declaration macros. See :ref:`qom` for more details. ATB, Mark.
Alex Bennée <alex.bennee@linaro.org> wrote: > We have a more complete document on QOM but we should at least mention > the style requirements in the style guide. > > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> Reviewed-by: Juan Quintela <quintela@redhat.com>
On 20/4/23 21:32, Mark Cave-Ayland wrote: > On 20/04/2023 16:57, Alex Bennée wrote: > >> We have a more complete document on QOM but we should at least mention >> the style requirements in the style guide. >> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >> Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> >> --- >> docs/devel/qom.rst | 2 ++ >> docs/devel/style.rst | 29 +++++++++++++++++++++++++++++ >> 2 files changed, 31 insertions(+) > A couple of points: > > 1) It is probably worth removing the typedefs given that they are > handled by the various QOM macros > > 2) There should be mention of the fixed names "parent_obj" and > "parent_class" for > the first declaration. > > How about something like this: > > > QEMU Object Model Declarations > ============================== > > The QEMU Object Model (QOM) provides a framework for handling objects > in the base C language. The first declaration of a storage or class > structure should always be the parent and leave a visual space between s/should/must/ > that declaration and the new code. > > For a storage structure the first declaration should always be called > "parent_obj" and for a class structure the first member should always > be called "parent_class" as below: > > .. code-block:: c > > struct MyDeviceState { > DeviceState parent_obj; > > /* Properties */ > int prop_a; > char *prob_b; Should we mention "We recommend placing instance/class properties fields just after the parent field"? > /* Other stuff */ > int internal_state; > }; > > struct MyDeviceClass { > DeviceClass parent_class; > > void (*new_fn1)(void); > bool (*new_fn2)(CPUState *); > }; > > Note that there is no need to provide typedefs for QOM structures since > these are generated automatically by the QOM declaration macros. See > :ref:`qom` for more details. > > > ATB, > > Mark.
On 21/04/2023 07:15, Philippe Mathieu-Daudé wrote: > On 20/4/23 21:32, Mark Cave-Ayland wrote: >> On 20/04/2023 16:57, Alex Bennée wrote: >> >>> We have a more complete document on QOM but we should at least mention >>> the style requirements in the style guide. >>> >>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >>> Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> >>> --- >>> docs/devel/qom.rst | 2 ++ >>> docs/devel/style.rst | 29 +++++++++++++++++++++++++++++ >>> 2 files changed, 31 insertions(+) > > >> A couple of points: >> >> 1) It is probably worth removing the typedefs given that they are handled by the >> various QOM macros >> >> 2) There should be mention of the fixed names "parent_obj" and "parent_class" for >> the first declaration. >> >> How about something like this: >> >> >> QEMU Object Model Declarations >> ============================== >> >> The QEMU Object Model (QOM) provides a framework for handling objects >> in the base C language. The first declaration of a storage or class >> structure should always be the parent and leave a visual space between > > s/should/must/ > >> that declaration and the new code. >> >> For a storage structure the first declaration should always be called >> "parent_obj" and for a class structure the first member should always >> be called "parent_class" as below: >> >> .. code-block:: c >> >> struct MyDeviceState { >> DeviceState parent_obj; >> >> /* Properties */ >> int prop_a; >> char *prob_b; > > Should we mention "We recommend placing instance/class properties fields > just after the parent field"? I don't think I've ever seen any recommendations on placing instance/class property fields other than for the parent DeviceState/DeviceClass? IMO it doesn't seem worth committing to anything else for now, especially as which fields get exposed as properties can be quite fluid these days ;) >> /* Other stuff */ >> int internal_state; >> }; >> >> struct MyDeviceClass { >> DeviceClass parent_class; >> >> void (*new_fn1)(void); >> bool (*new_fn2)(CPUState *); >> }; >> >> Note that there is no need to provide typedefs for QOM structures since these are >> generated automatically by the QOM declaration macros. See :ref:`qom` for more >> details. ATB, Mark.
diff --git a/docs/devel/qom.rst b/docs/devel/qom.rst index 3e34b07c98..c9237950d0 100644 --- a/docs/devel/qom.rst +++ b/docs/devel/qom.rst @@ -1,3 +1,5 @@ +.. _qom: + =========================== The QEMU Object Model (QOM) =========================== diff --git a/docs/devel/style.rst b/docs/devel/style.rst index 5bc6f2f095..0bd01f3fca 100644 --- a/docs/devel/style.rst +++ b/docs/devel/style.rst @@ -628,6 +628,35 @@ are still some caveats to beware of QEMU Specific Idioms ******************** +QEMU Object Model Declarations +============================== + +The QEMU Object Model (QOM) provides a framework for handling objects +in the base C language. The first declaration of a storage or class +structure should always be the parent and leave a visual space between +that declaration and the new code. + +.. code-block:: c + + typedef struct MyDeviceState { + DeviceState parent_obj; + + /* Properties */ + int prop_a; + char *prob_b; + /* Other stuff */ + int internal_state; + } MyDeviceState; + + typedef struct MyDeviceClass { + ObjectClass parent_class; + + void (*new_fn1)(void); + bool (*new_fn2)(CPUState *); + } MyDeviceClass; + +See :ref:`qom` for more details. + Error handling and reporting ============================
We have a more complete document on QOM but we should at least mention the style requirements in the style guide. Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> --- docs/devel/qom.rst | 2 ++ docs/devel/style.rst | 29 +++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+)