Zhao Liu <zhao1....@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 <arm...@redhat.com> >> Subject: Re: [PATCH] hw/core: Rename CpuTopology to CPUTopology >> >> Zhao Liu <zhao1....@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 <arm...@redhat.com> >> >> Subject: Re: [PATCH] hw/core: Rename CpuTopology to CPUTopology >> >> >> >> Zhao Liu <zhao1....@intel.com> writes: >> >> >> >> > Use CPUTopology to honor the generic style of CPU capitalization >> >> > abbreviations. >> >> > >> >> > Signed-off-by: Zhao Liu <zhao1....@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.