Re: [PATCH bpf-next v13 04/14] bpf: add struct_ops_tab to btf.

[Kui-Feng Lee <sinquersw@gmail.com>]

On 12/14/23 18:22, Martin KaFai Lau wrote:
> On 12/8/23 4:26 PM, thinker.li@gmail.com wrote:
>> +const struct bpf_struct_ops_desc *btf_get_struct_ops(struct btf *btf, 
>> u32 *ret_cnt)
>> +{
>> +    if (!btf)
>> +        return NULL;
>> +    if (!btf->struct_ops_tab)
> 
>          *ret_cnt = 0;
> 
> unless the later patch checks the return value NULL before using *ret_cnt.
> Anyway, better to set *ret_cnt to 0 if the btf has no struct_ops.
> 
> The same should go for the "!btf" case above but I suspect the above 
> !btf check is unnecessary also and the caller should have checked for 
> !btf itself instead of expecting a list of struct_ops from a NULL btf. 
> Lets continue the review on the later patches for now to confirm where 
> the above !btf case might happen.

Checking callers, I didn't find anything that make btf here NULL so far.
It is safe to remove !btf check. For the same reason as assigning
*ret_cnt for safe, this check should be fine here as well, right?

I don't have strong opinion here. What I though is to keep the values
as it is without any side-effect if the function call fails and if
possible. And, the callers should not expect the callee to set some
specific values when a call fails.

> 
>> +        return NULL;
>> +
>> +    *ret_cnt = btf->struct_ops_tab->cnt;
>> +    return (const struct bpf_struct_ops_desc *)btf->struct_ops_tab->ops;
>> +}
>