On Mon, Nov 21, 2022 at 02:16:25AM +0530, Kumar Kartikeya Dwivedi wrote:
>
> > Also please get rid of special_kfunc_set and
> > and btf_id_set_contains(&special_kfunc_set, meta.func_id)
> > That additional check is unnecessary as well.
> > special_kfunc_list is enough.
>
> It provides an easy way to do 'btf == vmlinux && is_special_kfunc'.
> Otherwise if I drop it, every check matching func_id == special_kfunc_list will
> also have to do btf == vmlinux check (because eventually we want to drop to the
> other branches that should work for non-vmlinux BTF as well).
>
> What I mean is:
> if (btf == vmlinux && btf_id_set_contains(special_kfunc_set)) {
> if (func_id == special_kfunc_list) {
> } else if (func_id == special_kfunc_list) {
> } else {
> }
> } else if (!__btf_type_is_struct) {
> } else /* struct */ {
> }
>
> will become:
> if (btf == vmlinux && func_id == special_kfunc_list) {
> } else if (btf == vmlinux && func_id == special_kfunc_list) {
> } else if (!__btf_type_is_struct) {
> } else /* struct */ {
> }
One less indent looks better.
Repeated btf == vmlinux doesn't bother me.
There is no use for this special_kfunc_set. It just wastes memory.
So I still think it's better to remove it.
While at it please swap branches !__btf_type_is_struct to __btf_type_is_struct:
else if (__btf_type_is_struct) {
regs[BPF_REG_0].type = PTR_TO_BTF_ID
} else { // handle 'void *' return aka r0_size
would read better.
