On Fri, May 05, 2017 at 11:06:02AM +0200, Andrew Jones wrote: > On Fri, May 05, 2017 at 10:09:18AM +0200, Igor Mammedov wrote: > > On Fri, 5 May 2017 11:45:22 +1000 > > David Gibson <da...@gibson.dropbear.id.au> wrote: > > > > > On Wed, May 03, 2017 at 02:57:06PM +0200, Igor Mammedov wrote: > > > > wrappers should make access to [has]node_id fields more readable > > > > > > > > Signed-off-by: Igor Mammedov <imamm...@redhat.com> > > > > > > Reviewed-by: David Gibson <da...@gibson.dropbear.id.au> > > > > > > Correct, though I'm not sure it actually simplifies things that much. > > > Maybe more in future patches, though. > > that's what Drew insisted on, and even though I prefer other way around > > I won't stall series arguing about styling issues, > > so here this patch goes. > > My argument in the last review of this series was that references like > > machine->possible_cpus->cpus[cs->cpu_index].props.has_node_id > and > machine->possible_cpus->cpus[cs->cpu_index].props.node_id > > are quite long, and only differ by 'has_', making it tough to > easily recognize.
If expression length is a problem, we can just use an extra variable: CPUArchId *slot = &machine->possible_cpus->cpus[cs->cpu_index]; if (slot->props.has_node_id && slot->props.node_id == FOO) ... > But, if nobody, but me, sees value in changing > them to > > numa_has_node_id(machine->possible_cpus, cs->cpu_index) > and > numa_node_id(machine->possible_cpus, cs->cpu_index) > > then I won't insist. I don't mind that much, but I still prefer the version without the wrappers. -- Eduardo
