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

[Markus Armbruster <armbru@redhat.com>]

Zhao Liu <zhao1.liu@intel.com> writes:

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

"Another fine mess"

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

Yes.