Re: [PATCH i-g-t] lib/igt_amd: improve link_settings parser for old kernel compatibility

[Harry Wentland <harry.wentland@amd.com>]

On 2026-09-09 16:44, Roman.Li@amd.com wrote:
> From: Roman Li <Roman.Li@amd.com>
> 
> The previous fix uses strstr() to locate each record label. However,
> strstr() stops at the first NUL byte (C string semantics), which breaks
> backward compatibility with old kernels that embed NUL bytes between
> records. Only the Current record would parse, leaving Verified, Reported,
> and Preferred unset (0), causing the same failure as the original bug.
> 
> Use memmem() instead to search the raw buffer across NUL boundaries,
> combined with sscanf() for parsing. This works with both:
>   - Old kernels: with embedded NUL separators
>   - New kernels: with contiguous single string
> 
> Also replace hardcoded loop bound '4' with ARRAY_SIZE(labels) for
> better maintainability.
> 
> Tested on eDP with both kernel variants: amd_ilr passes in both cases.
> 
> Fixes: f234bbd294f5 ("lib/igt_amd: don't rely on NUL separators parsing link_settings")
> Cc: Harry Wentland <harry.wentland@amd.com>
> Assisted-by: Claude Haiku 4.5
> Signed-off-by: Roman Li <Roman.Li@amd.com>

Reviewed-by: Harry Wentland <harry.wentland@amd.com>

Harry

> ---
>  lib/igt_amd.c | 35 ++++++++++++++++++++---------------
>  1 file changed, 20 insertions(+), 15 deletions(-)
> 
> diff --git a/lib/igt_amd.c b/lib/igt_amd.c
> index 7d33ab658..ba1cfabcf 100644
> --- a/lib/igt_amd.c
> +++ b/lib/igt_amd.c
> @@ -836,10 +836,8 @@ void igt_amd_read_link_settings(
>  	static const char * const labels[] = {
>  		"Current:", "Verified:", "Reported:", "Preferred:"
>  	};
> -	int fd, ret;
> +	int fd, ret, i;
>  	char buf[101];
> -	char *ptr;
> -	int i;
>  
>  	fd = igt_debugfs_connector_dir(drm_fd, connector_name, O_RDONLY);
>  	if (fd < 0) {
> @@ -853,22 +851,29 @@ void igt_amd_read_link_settings(
>  
>  	close(fd);
>  
> -	/* The debugfs node returns a single NUL-terminated string of the form:
> +	/* The debugfs node returns a string of the form:
>  	 *   "Current:  %d  0x%x  %d  Verified:  %d  0x%x  %d  "
>  	 *   "Reported:  %d  0x%x  %d  Preferred:  %d  0x%x  %d\n"
> -	 * Locate each label and parse the three values that follow it. Do not
> -	 * rely on embedded NUL bytes (or any other delimiter) between records,
> -	 * as the kernel emits a single contiguous string. */
> -	ptr = buf;
> -	for (i = 0; i < 4; i++) {
> -		ptr = strstr(ptr, labels[i]);
> -		if (ptr == NULL)
> +	 * Old kernels embed NUL bytes between records; new kernels emit a
> +	 * contiguous string. Use memmem() to locate each label across NUL
> +	 * boundaries and sscanf() to parse values, so both layouts work.
> +	 */
> +	for (i = 0; i < ARRAY_SIZE(labels); i++) {
> +		const char *label = labels[i];
> +		char *rec = memmem(buf, ret, label, strlen(label));
> +		unsigned int rate;
> +		int lanes, spread;
> +
> +		if (!rec)
> +			break;
> +
> +		if (sscanf(rec + strlen(label), "%d %x %d",
> +			   &lanes, &rate, &spread) != 3)
>  			break;
>  
> -		ptr += strlen(labels[i]);
> -		lane_count[i] = strtol(ptr, &ptr, 10);
> -		link_rate[i] = strtol(ptr, &ptr, 16);
> -		link_spread[i] = strtol(ptr, &ptr, 10);
> +		lane_count[i] = lanes;
> +		link_rate[i] = rate;
> +		link_spread[i] = spread;
>  	}
>  }
>