Re: [PATCH v3 01/14] hw/char/serial-pci: Replace DO_UPCAST(PCISerialState) by PCI_SERIAL()

[Philippe Mathieu-Daudé]

On 16/2/23 12:20, Thomas Huth wrote:
> On 13/02/2023 19.43, Philippe Mathieu-Daudé wrote:
> > Use the PCI_SERIAL() QOM type-checking macro to avoid DO_UPCAST().
> >
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > ---
> >   hw/char/serial-pci.c | 7 +++++--
> >   1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/char/serial-pci.c b/hw/char/serial-pci.c
> > index 801b769aba..9689645cac 100644
> > --- a/hw/char/serial-pci.c
> > +++ b/hw/char/serial-pci.c
> > @@ -36,7 +36,10 @@
> >   #include "qom/object.h"
> >   struct PCISerialState {
> > +    /*< private >*/
> >       PCIDevice dev;
> > +    /*< public >*/
> > +
>
> I'm not sure about this part of the patch. It does not seem to be 
> related to the other changes at all, and are you sure about which parts 
> are really "public" and which parts are "private"? If so, I'd like to 
> see a description about this in the commit message, preferably in a 
> separate patch. Also, why an empty line after the "public" comment?

This is how QOM style separates the object 'private' part -- the
inherited parent, used by the QOM-cast macros -- and the fields
specific to this object.
The private field *must* be the first one in the structure for the
cast macros to work.

Maybe this isn't a convention and we could make one, to unify the
API style. I'm open to better suggestion :)

I suppose I got custom to see it to distinct the QOM hierarchy and
now it helps me to detect what is QOM and what isn't.
Anyway I'll remove from this patch.


> Ack for the DO_UPCAST removal.

Thanks!