Re: [PATCH] hw/core: Rename CpuTopology to CPUTopology

[Zhao Liu]

On Tue, Jun 04, 2024 at 07:29:15AM +0200, Markus Armbruster wrote:
> Date: Tue, 04 Jun 2024 07:29:15 +0200
> From: Markus Armbruster <armbru@redhat.com>
> Subject: Re: [PATCH] hw/core: Rename CpuTopology to CPUTopology
> 
> Zhao Liu <zhao1.liu@intel.com> writes:
> 
> > On Mon, Jun 03, 2024 at 01:54:15PM +0200, Markus Armbruster wrote:
> >> Date: Mon, 03 Jun 2024 13:54:15 +0200
> >> From: Markus Armbruster <armbru@redhat.com>
> >> Subject: Re: [PATCH] hw/core: Rename CpuTopology to CPUTopology
> >> 
> >> Zhao Liu <zhao1.liu@intel.com> writes:
> >> 
> >> > Use CPUTopology to honor the generic style of CPU capitalization
> >> > abbreviations.
> >> >
> >> > Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> >> 
> >> Is CPUFoo really more common than CpuFoo?  It isn't in the qapi
> >> schema...
> >
> > Hi Markus, do you think this style needs to be standardized?
> >
> > All-caps cases, like the widely used CPUState.
> >
> > And the common structures declared in include/qemu/typedefs.h, are all
> > using CPU, not Cpu...
> >
> > If you feel this is necessary, I'd be happy to help more places change
> > their names to standardize their style...
> 
> The situation is unfortunate[*].  The renaming cure could be worse than
> the disease, though.
> 
> In a situation like this, settling for local consistency is often the
> least bad.  machine.json is locally consistent: it consistently uses
> CpuFoo.
> 
> 
> [*] We suck at systematic, disciplined naming.

I see, by local consistency principle, my another patch (adding topology
enumeration in machine.json) should use Cpu...

The CpuTopology that this patch modifies is located in include/hw/boards.h,
where that looks as if this file prefers to use CPUs (defining the
CPUArchIdList and CPUArchId). And there's also another case for all-caps,
SMPCompatProps (using SMP not Smp). So I feel like this patch change
still makes sense... Sorry if I'm being a bit obsessive.

The most confusing thing in include/hw/boards.h is this structure:

typedef struct CPUArchId {
    ...
    CpuInstanceProperties props;
    CPUState *cpu;
    ...
} CPUArchId;

CPU and Cpu are mixed together, but this is also explained by the local
consistency principle, since the CpuInstanceProperties belong to
machine.json. ;-)