diff mbox series

[v4,02/24] q800: add missing space after parent object in GLUEState

Message ID 20230621085353.113233-3-mark.cave-ayland@ilande.co.uk
State New
Headers show
Series q800: add support for booting MacOS Classic - part 1 | expand

Commit Message

Mark Cave-Ayland June 21, 2023, 8:53 a.m. UTC
This brings GLUEState in line with our current QOM guidelines.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 hw/m68k/q800.c | 1 +
 1 file changed, 1 insertion(+)

Comments

BALATON Zoltan June 21, 2023, 11:41 a.m. UTC | #1
On Wed, 21 Jun 2023, Mark Cave-Ayland wrote:
> This brings GLUEState in line with our current QOM guidelines.

Are these guidelines documented somewhere? I like this better than the 
public/private comments (although I prefer no space at all with just 
documenting that QOM object parents should not be accessed directly) but I 
haven't seen it discussed and agreed upon so it looks like a convention 
you defined but not documented anywhere. But it could be I missed the 
patch to coding style or QOM docs to establish this convention.

If we really want to make these QOM object states stand out we might even 
consider formatting these as

struct GLUEState { SysBusDevice parent_obj;
     M68kCPU *cpu;
     ...
}

unless checkpatch would not like that or something.

Regards,
BALATON Zoltan

> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
> ---
> hw/m68k/q800.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
> index dda57c60bf..465c510c18 100644
> --- a/hw/m68k/q800.c
> +++ b/hw/m68k/q800.c
> @@ -100,6 +100,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(GLUEState, GLUE)
>
> struct GLUEState {
>     SysBusDevice parent_obj;
> +
>     M68kCPU *cpu;
>     uint8_t ipr;
>     uint8_t auxmode;
>
Mark Cave-Ayland June 21, 2023, 2:24 p.m. UTC | #2
On 21/06/2023 12:41, BALATON Zoltan wrote:

> On Wed, 21 Jun 2023, Mark Cave-Ayland wrote:
>> This brings GLUEState in line with our current QOM guidelines.
> 
> Are these guidelines documented somewhere? I like this better than the public/private 
> comments (although I prefer no space at all with just documenting that QOM object 
> parents should not be accessed directly) but I haven't seen it discussed and agreed 
> upon so it looks like a convention you defined but not documented anywhere. But it 
> could be I missed the patch to coding style or QOM docs to establish this convention.

Alex documented this earlier in the year: you can find this online at 
https://qemu.readthedocs.io/en/master/devel/style.html#qemu-specific-idioms.

> If we really want to make these QOM object states stand out we might even consider 
> formatting these as
> 
> struct GLUEState { SysBusDevice parent_obj;
>      M68kCPU *cpu;
>      ...
> }
> 
> unless checkpatch would not like that or something.

I'm not overly convinced by this, and yes I suspect it would also require some 
hacking on checkpatch.pl for it to work.


ATB,

Mark.
BALATON Zoltan June 21, 2023, 4:12 p.m. UTC | #3
On Wed, 21 Jun 2023, Mark Cave-Ayland wrote:
> On 21/06/2023 12:41, BALATON Zoltan wrote:
>
>> On Wed, 21 Jun 2023, Mark Cave-Ayland wrote:
>>> This brings GLUEState in line with our current QOM guidelines.
>> 
>> Are these guidelines documented somewhere? I like this better than the 
>> public/private comments (although I prefer no space at all with just 
>> documenting that QOM object parents should not be accessed directly) but I 
>> haven't seen it discussed and agreed upon so it looks like a convention you 
>> defined but not documented anywhere. But it could be I missed the patch to 
>> coding style or QOM docs to establish this convention.
>
> Alex documented this earlier in the year: you can find this online at 
> https://qemu.readthedocs.io/en/master/devel/style.html#qemu-specific-idioms.

The examples in
https://qemu.readthedocs.io/en/master/devel/qom.html
now contradict that by using parent instead of parent_obj there.

>> If we really want to make these QOM object states stand out we might even 
>> consider formatting these as
>> 
>> struct GLUEState { SysBusDevice parent_obj;
>>      M68kCPU *cpu;
>>      ...
>> }
>> 
>> unless checkpatch would not like that or something.
>
> I'm not overly convinced by this, and yes I suspect it would also require 
> some hacking on checkpatch.pl for it to work.

Nevermind, was just an idea. Blank line wiithout comments is also a good 
convention and less weird looking than the above.

Regards,
BALATON Zoltan

>
>
> ATB,
>
> Mark.
>
>
diff mbox series

Patch

diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index dda57c60bf..465c510c18 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -100,6 +100,7 @@  OBJECT_DECLARE_SIMPLE_TYPE(GLUEState, GLUE)
 
 struct GLUEState {
     SysBusDevice parent_obj;
+
     M68kCPU *cpu;
     uint8_t ipr;
     uint8_t auxmode;