On Wed, 16 Dec 2020 at 06:44, Andrew Jones <drjo...@redhat.com> wrote: > > On Tue, Dec 15, 2020 at 06:20:48PM +0000, David Edmondson wrote: > > On Tuesday, 2020-12-15 at 18:48:15 +01, Andrew Jones wrote: > > > > > virt machine's 'smp_cpus' and machine->smp.cpus must always have the > > > same value. And, anywhere we have virt machine state we have machine > > > state. So let's remove the redundancy. Also, to make it easier to see > > > that machine->smp is the true source for "smp_cpus" and "max_cpus", > > > avoid passing them in function parameters, preferring instead to get > > > them from the state.
> > > static void fdt_add_cpu_nodes(const VirtMachineState *vms) > > > { > > > - int cpu; > > > - int addr_cells = 1; > > > const MachineState *ms = MACHINE(vms); > > > + int smp_cpus = ms->smp.cpus, cpu; > > > > Is it house-style to have initialised and un-initialised local variables > > declared on the same line? > > > > checkpatch.pl doesn't complain and a grep of qemu shows hundreds of other > examples. That said, I only see one other example in hw/arm/virt.c, so if > we'd rather avoid it, I'll repost. I think this is one of those things where the style guide doesn't say anything, so it comes down to individual developer preference. Personally I find declaring an uninitialized local on the same line and after an initialized local is a bit confusing to read so I've tweaked the patch, but it's not a big deal either way. Applied to target-arm.next, thanks. -- PMM