Re: [PATCH bpf-next v2] selftests/bpf: support struct/union presets in veristat

[Eduard Zingerman <eddyz87@gmail.com>]

On Mon, 2025-03-24 at 12:34 +0000, Mykyta Yatsenko wrote:
> From: Mykyta Yatsenko <yatsenko@meta.com>
> 
> Extend commit e3c9abd0d14b ("selftests/bpf: Implement setting global
> variables in veristat") to support applying presets to members of
> the global structs or unions in veristat.
> For example:
> ```
> ./veristat set_global_vars.bpf.o  -G "union1.struct3.var_u8_h = 0xBB"
> ```
> 
> Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
> ---

Thank you for addressing comments from my previous review.
Sorry for the delay, didn't look at patches while travelling.

Acked-by: Eduard Zingerman <eddyz87@gmail.com>

[...]

> +static int adjust_var_secinfo_tok(char **name_tok, const struct btf *btf,
> +				  const struct btf_type *t, struct btf_var_secinfo *sinfo)
> +{
> +	char *name = strtok_r(NULL, ".", name_tok);
> +	const struct btf_type *member_type;
> +	const struct btf_member *member;
> +	int member_tid;
> +	__u32 anon_offset = 0;
> +
> +	if (!name)
> +		return 0;
> +
> +	member = btf_find_member(btf, t, name, &anon_offset);
> +	if (IS_ERR(member)) {
> +		fprintf(stderr, "Could not find member %s\n", name);
> +		return -EINVAL;
> +	}
> +
> +	member_tid = btf__resolve_type(btf, member->type);
> +	member_type = btf__type_by_id(btf, member_tid);
> +
> +	if (btf_kflag(t)) {
> +		fprintf(stderr, "Bitfield presets are not supported %s\n", name);
> +		return -EINVAL;
> +	}
> +	sinfo->offset += (member->offset + anon_offset) / 8;
> +	sinfo->size = member_type->size;
> +	sinfo->type = member_tid;

Nit: I'd push this assignment down to `btf_find_member` and avoid
     `&anon_offset` parameter. It would be easier to read this way,
     as all offset manipulations would be in one place.

> +
> +	return adjust_var_secinfo_tok(name_tok, btf, member_type, sinfo);
> +}
> +
> +static int adjust_var_secinfo(struct btf *btf, const struct btf_type *t,
> +			      struct btf_var_secinfo *sinfo, const char *var)
> +{
> +	char expr[256], *saveptr;
> +	const struct btf_type *base_type;
> +	int err;
> +
> +	base_type = btf__type_by_id(btf, btf__resolve_type(btf, t->type));
> +	strncpy(expr, var, 256);

Nit: strncpy does not null terminate if strlen(src) == N.

> +	strtok_r(expr, ".", &saveptr);
> +	err = adjust_var_secinfo_tok(&saveptr, btf, base_type, sinfo);
> +	if (err)
> +		return err;
> +
> +	return 0;
> +}
> +
> +static int set_global_var(struct bpf_object *obj, struct btf *btf,
>  			  struct bpf_map *map, struct btf_var_secinfo *sinfo,
>  			  struct var_preset *preset)
>  {

[...]