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

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

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!