On Mon, Apr 10, 2023 at 10:46 AM Barret Rhoden <brho@xxxxxxxxxx> wrote:
>
> On 3/28/23 17:32, Andrii Nakryiko wrote:
> > This one looks pretty useful and common (especially to work around
> > Clang's smartness). I have related set of helpers waiting it's time,
> > see [0]. I'd say we should think about some good naming of them,
> > document them properly (including various gotchas), and include them
> > (initially) in bpf_misc.h and start using them in selftests.
>
> sounds good to me.
>
> any preference for a name? bpf_index_array()?
> - no need to say "bounded".
> - want to begin with bpf_.
> - sticking with your clamp style, "index" would be the verb. (instead
> of e.g. bpf_array_index, which sounds like it has something to do with a
> BPF array".
>
> i'm up for anything though. =)
bpf_index_array()/bpf_access_array() might be ok. But perhaps a bit
more natural in the code will be bpf_array_elem(), as it's actually an
accessor/expression, not really a stand-alone statement:
struct my_elem *elem = bpf_array_elem(arr, n);
if (!elem)
return 0;
elem->field = <blah>;
WDYT?
Let's also maybe improve usability by inferring array size? I've seen
somewhere in kernel code a macro that validates that passed in arr is
actually an array (and not a pointer), it would be useful to use
similar approach here to make sure we determine size correctly. See
what ARRAY_SIZE() in the kernel does.
>
> > yeah, I'm hesitant about this one. It is very similar to
> > bpf_clam_xxx() macros I referenced above, probably we should use those
> > instead.
>
> totally agree. i can switch my stuff to use the clamp, which covers all
> of the use cases/signedness. btw, good to know the "s<" is
> signed-less-than. i imagine i'd have had trouble with that. =)
you are welcome :)
>
> thanks,
>
> barret
>
>
