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 |
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; >
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.
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 --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;