[PATCH bpf-next] selftests/bpf: improve error messages in veristat

[PATCH bpf-next] selftests/bpf: improve error messages in veristat

[Mykyta Yatsenko]

From: Mykyta Yatsenko <yatsenko@meta.com>

Return error if preset parsing fails. Avoid proceeding with veristat run
if preset does not parse.
Before:
```
./veristat set_global_vars.bpf.o -G "arr[999999999999999999999] = 1"
Failed to parse value '999999999999999999999'
Processing 'set_global_vars.bpf.o'...
File                   Program           Verdict  Duration (us)  Insns  States  Program size  Jited size
---------------------  ----------------  -------  -------------  -----  ------  ------------  ----------
set_global_vars.bpf.o  test_set_globals  success             27     64       0            82           0
---------------------  ----------------  -------  -------------  -----  ------  ------------  ----------
Done. Processed 1 files, 0 programs. Skipped 1 files, 0 programs.
```
After:
```
./veristat set_global_vars.bpf.o -G "arr[999999999999999999999] = 1"
Failed to parse value '999999999999999999999'
Failed to parse global variable presets: arr[999999999999999999999] = 1
```

Improve error messages:
 * If preset struct member can't be found.
 * Array index out of bounds

Extract rtrim function.

Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
---
 tools/testing/selftests/bpf/veristat.c | 36 ++++++++++++++++++--------
 1 file changed, 25 insertions(+), 11 deletions(-)

diff --git a/tools/testing/selftests/bpf/veristat.c b/tools/testing/selftests/bpf/veristat.c
index 1e9f61f9fd0a..09cfbd486f92 100644
--- a/tools/testing/selftests/bpf/veristat.c
+++ b/tools/testing/selftests/bpf/veristat.c
@@ -890,6 +890,18 @@ static bool is_desc_sym(char c)
 	return c == 'v' || c == 'V' || c == '.' || c == '!' || c == '_';
 }
 
+static char *rtrim(char *str)
+{
+	int i;
+
+	for (i = strlen(str) - 1; i > 0; --i) {
+		if (!isspace(str[i]))
+			break;
+		str[i] = '\0';
+	}
+	return str;
+}
+
 static int parse_stat(const char *stat_name, struct stat_specs *specs)
 {
 	int id;
@@ -1666,7 +1678,7 @@ static int append_preset_atom(struct var_preset *preset, char *value, bool is_in
 static int parse_var_atoms(const char *full_var, struct var_preset *preset)
 {
 	char expr[256], var[256], *name, *saveptr;
-	int n, len, off;
+	int n, len, off, err;
 
 	snprintf(expr, sizeof(expr), "%s", full_var);
 	preset->atom_count = 0;
@@ -1677,7 +1689,9 @@ static int parse_var_atoms(const char *full_var, struct var_preset *preset)
 			fprintf(stderr, "Can't parse %s\n", name);
 			return -EINVAL;
 		}
-		append_preset_atom(preset, var, false);
+		err = append_preset_atom(preset, var, false);
+		if (err)
+			return err;
 
 		/* parse optional array indexes */
 		while (off < len) {
@@ -1685,7 +1699,9 @@ static int parse_var_atoms(const char *full_var, struct var_preset *preset)
 				fprintf(stderr, "Can't parse %s as index\n", name + off);
 				return -EINVAL;
 			}
-			append_preset_atom(preset, var, true);
+			err = append_preset_atom(preset, var, true);
+			if (err)
+				return err;
 			off += n;
 		}
 	}
@@ -1697,7 +1713,7 @@ static int append_var_preset(struct var_preset **presets, int *cnt, const char *
 	void *tmp;
 	struct var_preset *cur;
 	char var[256], val[256];
-	int n, err, i;
+	int n, err;
 
 	tmp = realloc(*presets, (*cnt + 1) * sizeof(**presets));
 	if (!tmp)
@@ -1712,11 +1728,7 @@ static int append_var_preset(struct var_preset **presets, int *cnt, const char *
 		return -EINVAL;
 	}
 	/* Remove trailing spaces from var, as scanf may add those */
-	for (i = strlen(var) - 1; i > 0; --i) {
-		if (!isspace(var[i]))
-			break;
-		var[i] = '\0';
-	}
+	rtrim(var);
 
 	err = parse_rvalue(val, &cur->value);
 	if (err)
@@ -1869,7 +1881,7 @@ static int adjust_var_secinfo_array(struct btf *btf, int tid, struct field_acces
 	if (err)
 		return err;
 	if (idx < 0 || idx >= barr->nelems) {
-		fprintf(stderr, "Array index %lld is out of bounds [0, %u]: %s\n",
+		fprintf(stderr, "Array index %lld is out of bounds [0, %u): %s\n",
 			idx, barr->nelems, array_name);
 		return -EINVAL;
 	}
@@ -1928,7 +1940,7 @@ static int adjust_var_secinfo_member(const struct btf *btf,
 		}
 	}
 
-	return -EINVAL;
+	return -ESRCH;
 }
 
 static int adjust_var_secinfo(struct btf *btf, const struct btf_type *t,
@@ -1955,6 +1967,8 @@ static int adjust_var_secinfo(struct btf *btf, const struct btf_type *t,
 			break;
 		case FIELD_NAME:
 			err = adjust_var_secinfo_member(btf, base_type, 0, atom->name, sinfo);
+			if (err == -ESRCH)
+				fprintf(stderr, "Can't find '%s'\n", atom->name);
 			prev_name = atom->name;
 			break;
 		default:
-- 
2.50.0

Re: [PATCH bpf-next] selftests/bpf: improve error messages in veristat

[Eduard Zingerman]

On Fri, 2025-06-27 at 15:43 +0100, Mykyta Yatsenko wrote:
> From: Mykyta Yatsenko <yatsenko@meta.com>
> 
> Return error if preset parsing fails. Avoid proceeding with veristat run
> if preset does not parse.
> Before:
> ```
> ./veristat set_global_vars.bpf.o -G "arr[999999999999999999999] = 1"
> Failed to parse value '999999999999999999999'
> Processing 'set_global_vars.bpf.o'...
> File                   Program           Verdict  Duration (us)  Insns  States  Program size  Jited size
> ---------------------  ----------------  -------  -------------  -----  ------  ------------  ----------
> set_global_vars.bpf.o  test_set_globals  success             27     64       0            82           0
> ---------------------  ----------------  -------  -------------  -----  ------  ------------  ----------
> Done. Processed 1 files, 0 programs. Skipped 1 files, 0 programs.
> ```
> After:
> ```
> ./veristat set_global_vars.bpf.o -G "arr[999999999999999999999] = 1"
> Failed to parse value '999999999999999999999'
> Failed to parse global variable presets: arr[999999999999999999999] = 1
> ```
> 
> Improve error messages:
>  * If preset struct member can't be found.
>  * Array index out of bounds
> 
> Extract rtrim function.
> 
> Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
> ---

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

> @@ -1955,6 +1967,8 @@ static int adjust_var_secinfo(struct btf *btf, const struct btf_type *t,
>  			break;
>  		case FIELD_NAME:
>  			err = adjust_var_secinfo_member(btf, base_type, 0, atom->name, sinfo);
> +			if (err == -ESRCH)
> +				fprintf(stderr, "Can't find '%s'\n", atom->name);

Nit: adjust_var_secinfo_member() already reports a few errors,
     maybe report this error there as well?

>  			prev_name = atom->name;
>  			break;
>  		default:

Re: [PATCH bpf-next] selftests/bpf: improve error messages in veristat

[Mykyta Yatsenko]

On 6/27/25 20:12, Eduard Zingerman wrote:
> On Fri, 2025-06-27 at 15:43 +0100, Mykyta Yatsenko wrote:
>> From: Mykyta Yatsenko <yatsenko@meta.com>
>>
>> Return error if preset parsing fails. Avoid proceeding with veristat run
>> if preset does not parse.
>> Before:
>> ```
>> ./veristat set_global_vars.bpf.o -G "arr[999999999999999999999] = 1"
>> Failed to parse value '999999999999999999999'
>> Processing 'set_global_vars.bpf.o'...
>> File                   Program           Verdict  Duration (us)  Insns  States  Program size  Jited size
>> ---------------------  ----------------  -------  -------------  -----  ------  ------------  ----------
>> set_global_vars.bpf.o  test_set_globals  success             27     64       0            82           0
>> ---------------------  ----------------  -------  -------------  -----  ------  ------------  ----------
>> Done. Processed 1 files, 0 programs. Skipped 1 files, 0 programs.
>> ```
>> After:
>> ```
>> ./veristat set_global_vars.bpf.o -G "arr[999999999999999999999] = 1"
>> Failed to parse value '999999999999999999999'
>> Failed to parse global variable presets: arr[999999999999999999999] = 1
>> ```
>>
>> Improve error messages:
>>   * If preset struct member can't be found.
>>   * Array index out of bounds
>>
>> Extract rtrim function.
>>
>> Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
>> ---
> Acked-by: Eduard Zingerman <eddyz87@gmail.com>
>
>> @@ -1955,6 +1967,8 @@ static int adjust_var_secinfo(struct btf *btf, const struct btf_type *t,
>>   			break;
>>   		case FIELD_NAME:
>>   			err = adjust_var_secinfo_member(btf, base_type, 0, atom->name, sinfo);
>> +			if (err == -ESRCH)
>> +				fprintf(stderr, "Can't find '%s'\n", atom->name);
> Nit: adjust_var_secinfo_member() already reports a few errors,
>       maybe report this error there as well?
That was my first attempt, but adjust_var_secinfo_member is called 
recursively for anonymous embedded structures, so this particular error 
will be noisy, as it'll get triggered for every embedded structure. 
Placing error msg here makes sure it's printed just once and only in 
cases when member is not present anywhere.
>
>>   			prev_name = atom->name;
>>   			break;
>>   		default:

Re: [PATCH bpf-next] selftests/bpf: improve error messages in veristat

[Eduard Zingerman]

On Fri, 2025-06-27 at 21:01 +0100, Mykyta Yatsenko wrote:

[...]

> > > @@ -1955,6 +1967,8 @@ static int adjust_var_secinfo(struct btf *btf, const struct btf_type *t,
> > >   			break;
> > >   		case FIELD_NAME:
> > >   			err = adjust_var_secinfo_member(btf, base_type, 0, atom->name, sinfo);
> > > +			if (err == -ESRCH)
> > > +				fprintf(stderr, "Can't find '%s'\n", atom->name);
> > Nit: adjust_var_secinfo_member() already reports a few errors,
> >       maybe report this error there as well?

> That was my first attempt, but adjust_var_secinfo_member is called 
> recursively for anonymous embedded structures, so this particular error 
> will be noisy, as it'll get triggered for every embedded structure. 
> Placing error msg here makes sure it's printed just once and only in 
> cases when member is not present anywhere.

Makes sense, thank you for explaining.

Re: [PATCH bpf-next] selftests/bpf: improve error messages in veristat

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <ast@kernel.org>:

On Fri, 27 Jun 2025 15:43:42 +0100 you wrote:
> From: Mykyta Yatsenko <yatsenko@meta.com>
> 
> Return error if preset parsing fails. Avoid proceeding with veristat run
> if preset does not parse.
> Before:
> ```
> ./veristat set_global_vars.bpf.o -G "arr[999999999999999999999] = 1"
> Failed to parse value '999999999999999999999'
> Processing 'set_global_vars.bpf.o'...
> File                   Program           Verdict  Duration (us)  Insns  States  Program size  Jited size
> 
> [...]

Here is the summary with links:
  - [bpf-next] selftests/bpf: improve error messages in veristat
    https://git.kernel.org/bpf/bpf-next/c/ffaff1804e2c

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html