On Sun, Mar 20, 2022 at 01:36:41AM +0530, Kumar Kartikeya Dwivedi wrote:
> > > > return -EINVAL;
> > > > +
> > > > + switch (field_type) {
> > > > + case BTF_FIELD_SPIN_LOCK:
> > > > + case BTF_FIELD_TIMER:
> > >
> > > Since spin_lock vs timer is passed into btf_find_struct_field() as field_type
> > > argument there is no need to pass name, sz, align from the caller.
> > > Pls make btf_find_spin_lock() to pass BTF_FIELD_SPIN_LOCK only
> > > and in the above code do something like:
> > > switch (field_type) {
> > > case BTF_FIELD_SPIN_LOCK:
> > > name = "bpf_spin_lock";
> > > sz = ...
> > > break;
> > > case BTF_FIELD_TIMER:
> > > name = "bpf_timer";
> > > sz = ...
> > > break;
> > > }
> >
> > Would doing this in btf_find_field be better? Then we set these once instead of
> > doing it twice in btf_find_struct_field, and btf_find_datasec_var.
yeah. probably.
> >
> > > switch (field_type) {
> > > case BTF_FIELD_SPIN_LOCK:
> > > case BTF_FIELD_TIMER:
> > > if (!__btf_type_is_struct(member_type))
> > > continue;
> > > if (strcmp(__btf_name_by_offset(btf, member_type->name_off), name))
> > > ...
> > > btf_find_field_struct(btf, member_type, off, sz, info);
> > > }
> > >
> > > It will cleanup the later patch which passes NULL, sizeof(u64), alignof(u64)
> > > only to pass something into the function.
> > > With above suggestion it wouldn't need to pass dummy args. BTF_FIELD_KPTR will be enough.
> > >
>
> Just to be clear, for the kptr case we still use size and align, only name is
> optional. size is used for datasec_var call, align is used in both struct_field
> and datasec_var. So I'm not sure whether moving it around has much effect,
> instead of the caller it will now be set based on field_type inside
> btf_find_field.
There is no use case to do BTF_FIELD_KPTR, sizeof(u64) and BTF_FIELD_KPTR, sizeof(u32), right?
So best to avoid such mistakes.
In other words consider every function to be a uapi.
Not in a way that it can never change, but from pov that you wouldn't want the user space
to specify all details for the kernel when BTF_FIELD_KPTR is enough to figure out the rest.
