On Fri, 5 May 2017 14:12:26 -0300 Eduardo Habkost <ehabk...@redhat.com> wrote:
> 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. considering 3vs1 for wrapper less variant, I'll return it back. but respin will have to wait till Tuesday due to holiday over here.
