Re: [PATCH 1/2] libbpf: add proto_func param name generation on sanitazing it to enum type

[Mykyta Yatsenko <mykyta.yatsenko5@gmail.com>]

On 31/03/2025 21:09, Timur Chernykh wrote:
> Signed-off-by: Timur Chernykh <tim.cherry.co@gmail.com>
Thanks for submitting this patchset, do you mind adding a proper commit 
message for each commit in the set.
> ---
>   tools/lib/bpf/libbpf.c | 42 ++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 42 insertions(+)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 6b85060f07b3..8e1edba443dd 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -3128,6 +3128,8 @@ static int bpf_object__sanitize_btf(struct bpf_object *obj, struct btf *btf)
>   	bool has_type_tag = kernel_supports(obj, FEAT_BTF_TYPE_TAG);
>   	bool has_enum64 = kernel_supports(obj, FEAT_BTF_ENUM64);
>   	bool has_qmark_datasec = kernel_supports(obj, FEAT_BTF_QMARK_DATASEC);
> +
> +	char name_gen_buff[32] = {0};
>   	int enum64_placeholder_id = 0;
>   	struct btf_type *t;
>   	int i, j, vlen;
> @@ -3178,10 +3180,50 @@ static int bpf_object__sanitize_btf(struct bpf_object *obj, struct btf *btf)
>   			if (name[0] == '?')
>   				name[0] = '_';
>   		} else if (!has_func && btf_is_func_proto(t)) {
> +			struct btf_param* params;
`struct btf_param *params` here and in other places, put asterisk closer 
to var name, not type.
> +			int new_param_name_off;
Maybe just `new_name_off`, prefer shorter names, when possible.
> +
>   			/* replace FUNC_PROTO with ENUM */
>   			vlen = btf_vlen(t);
>   			t->info = BTF_INFO_ENC(BTF_KIND_ENUM, 0, vlen);
>   			t->size = sizeof(__u32); /* kernel enforced */
> +
> +			/* since the btf_enum and btf_param has the same binary layout it's ok to use btf_param */
> +			params = btf_params(t);
> +
> +			for (j = 0; j < vlen; ++j) {
> +				struct btf_param* param = &params[j];
> +				const char* param_name = btf__str_by_offset(btf, param->name_off);
> +
> +				/*
> +				 * kernel disallow any unnamed enum members which can be generated for,
> +				 * as example, struct members like
> +				 * struct quota_format_ops {
> +				 *     ...
> +				 *     int (*get_next_id)(struct super_block *, struct kqid *);
> +				 *     ...
> +				 * }
> +				 */
> +				if (param_name && param_name[0]) {
> +					/* definitely has a name, valid it or no should decide kernel verifier */
> +					continue;
> +				}
> +
> +				/*
> +				 * generate an uniq name for each func_proto
> +				 */
> +				snprintf(name_gen_buff, sizeof(name_gen_buff), "__parm_proto_%d_%d", i, j);
> +				new_param_name_off = btf__add_str(btf, name_gen_buff);
> +
> +				if (new_param_name_off < 0) {
> +					pr_warn("Error creating the name for func_proto param");
append \n to the error message.
> +					return new_param_name_off;
> +				}
> +
> +				/* give a valid name to func_proto param as it now an enum member */
> +				param->name_off = new_param_name_off;
> +			}
> +
>   		} else if (!has_func && btf_is_func(t)) {
>   			/* replace FUNC with TYPEDEF */
>   			t->info = BTF_INFO_ENC(BTF_KIND_TYPEDEF, 0, 0);