On Thu, 2024-03-28 at 11:49 -0700, Jakub Kicinski wrote:
> > So in that sense, I prefer that, but I'm truly not sure how the (hand-
> > written) userspace code would deal with that.
>
> I think the best way today would be two walks:
>
> for_each_attr() {
> switch (type):
> case THE_A_ARRAY_1:
> cnt1++;
> break;
> case THE_A_ARRAY_2:
> cnt2++;
> break;
> }
>
> if (cnt1)
> array_1 = calloc();
> cnt1 = 0; /* we'll use it as index in second loop */
> if (cnt2)
> array_2 = calloc();
> cnt2 = 0;
>
> for_each_attr() {
> /* [ normal parsing, populating array_1[cnt1++] etc. ] */
> }
Yeah, that makes sense.
I'm not sure we even need the calloc() all the time, depends what we're
doing with it, of course.
> Compared to "indexed array" the only practical difference I think is
> the fact that all attrs are walked. I think you have to count them
> either way before parsing.
Right, generally the pattern would be something like
nla_for_each_nested(...)
n++;
// alloc etc.
idx = 0;
nla_for_each_nested(...)
array[idx++] = whatever(attr);
or something like that.
So I guess the only thing that changes really is that this now becomes
nla_for_each(...)
if (type != DESIRED)
continue;
vs.
nla_for_each_nested(...)
I suppose we could even define a
nla_for_each_type(..., type)
for that.
> I was wondering at some point whether we should require that all
> multi-attr attributes are grouped together. Or add an explicit "count"
> attribute. But couldn't convince myself that such extra rules will
> pay off sufficiently with perf and/or ease of use...
That doesn't seem likely, after all, you'll definitely want to double-
check all that ... Personally, unless you have something super perf
critical, I definitely prefer _not_ having a count like that in the API
because it encourages unsafe code that doesn't do the necessary bounds
checks and then crashes ...
johannes
