Messages in this thread | | | Date | Tue, 11 Jan 2022 10:14:25 -0500 | From | Steven Rostedt <> | Subject | Re: [PATCH -next, v2] sched: Use struct_size() helper in task_numa_group() |
| |
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
| |