Re: [PATCH i-g-t v2 4/8] tools/vbt_decode: validate DEVICE env var

[Jani Nikula <jani.nikula@intel.com>]

On Mon, 08 Jun 2026, Michał Grzelak <michal.grzelak@intel.com> wrote:
> On error, strotoul() returns converted value of longest substring
> spanning from beginning. Check if device ID input via DEVICE env var is
> valid. Log to the user if it is not and reset the devid.

The idea is good, nitpicks on the implementation.

>
> Signed-off-by: Michał Grzelak <michal.grzelak@intel.com>
> ---
>  tools/intel_vbt_decode.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/tools/intel_vbt_decode.c b/tools/intel_vbt_decode.c
> index 6a9c1de7b3..5681643f4b 100644
> --- a/tools/intel_vbt_decode.c
> +++ b/tools/intel_vbt_decode.c
> @@ -4315,8 +4315,13 @@ int main(int argc, char **argv)
>  
>  	if (!context.devid) {
>  		const char *devid_string = getenv("DEVICE");
> -		if (devid_string)
> -			context.devid = strtoul(devid_string, NULL, 16);
> +		if (devid_string) {
> +			context.devid = strtoul(devid_string, &endp, 16);
> +			if (*endp) {

	if (!context.devid || *endp)

> +				fprintf(stderr, "invalid devid: %s\n", endp);

This should print the whole devid_string, not endp. Most places also
have single quotes around %s for this kind of stuff.

Maybe this should also say where the invalid devid originates from?
invalid devid in DEVICE environment variable?

> +				context.devid = 0;

I think just return EXIT_FAILURE; here. No need to proceed.

> +			}
> +		}
>  	}
>  	if (!context.devid)
>  		context.devid = get_device_id(VBIOS, size);

-- 
Jani Nikula, Intel