Re: [PATCH -next, v2] sched: Use struct_size() helper in task_numa_group()

[Steven Rostedt <rostedt@goodmis.org>]

On Tue, 11 Jan 2022 12:30:42 +0100
Peter Zijlstra <peterz@infradead.org> wrote:

> > > >  	if (unlikely(!deref_curr_numa_group(p))) {
> > > > -		unsigned int size = sizeof(struct numa_group) +
> > > > -				    NR_NUMA_HINT_FAULT_STATS *
> > > > -				    nr_node_ids * sizeof(unsigned long);
> > > > +		unsigned int size = struct_size(grp, faults,
> > > > +						NR_NUMA_HINT_FAULT_STATS * nr_node_ids);    
> > > 
> > > Again, why?! The old code was perfectly readable, this, not so much.  
> > 
> > Because it is unsafe,  
> 
> Unsafe how? Changelog doesn't mention anything, nor do you. In fact,
> Changelog says there is no functional change, which makes me hate the
> thing for obscuring something that was simple.

If for some reason faults changes in size, the original code must be
updated whereas the new code is robust enough to not need changing.

> 
> > And to be honest, the new change is a lot easier to read than the original
> > code.  
> 
> I find it the other way around, because now I need to find and untangle
> the unholy mess that is struct_size(), whereas currently it is trivial
> C.

It's a C hack and far from trivial. Maybe to you as you are use to
these hacks. But seriously, this is not something the average C coder
is use to, as variable length structures are rather unique to the
kernel.

Note that struct_size() is commonly used in the kernel. Better start
getting use to it ;-)

-- Steve