Re: [PATCH i-g-t 02/10] lib/xe: Use stricter line-equality check when checking for workarounds

Matt Roper <[email protected]> writes:

> On Tue, Jul 21, 2026 at 03:59:55PM -0300, Gustavo Sousa wrote:
>> Currently debugfs_file_has_wa() uses strstr() to check if a workaround
>> name is present in the debugfs dump.  Using strstr() would match the
>> workaround name anywhere in the dump buffer and with that we risk
>> producing unexpected results if the checked workaround name happens to
>> be a substring of another workaround present in the dump.
>> 
>> Fix that by making sure we match the workaround name with the full
>> line from the dump.
>
> There are other parts of IGT that use regular expressions from either
> POSIX (regcomp / regexec) or Glib (g_regex_new / g_regex_match).  Would
> it be possible to use one of those here instead of opencoding a match
> function?

Yeah.  That could make this function more readable and easier to extend.
I'll take a look at those options.

--
Gustavo Sousa

>
>
> Matt
>
>> 
>> Signed-off-by: Gustavo Sousa <[email protected]>
>> ---
>>  lib/xe/xe_wa.c | 39 ++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 38 insertions(+), 1 deletion(-)
>> 
>> diff --git a/lib/xe/xe_wa.c b/lib/xe/xe_wa.c
>> index ff5daf529831..d44431e7e61f 100644
>> --- a/lib/xe/xe_wa.c
>> +++ b/lib/xe/xe_wa.c
>> @@ -13,6 +13,43 @@
>>  #include "xe/xe_wa.h"
>>  #include "xe/xe_query.h"
>>  
>> +static bool debugfs_dump_has_wa(char *dump, const char *wa)
>> +{
>> +	char *a = dump;
>> +
>> +	while (*a) {
>> +		const char *b = wa;
>> +
>> +		/*
>> +		 * Each workaround name is indented by one tab
>> +		 * character; unindented lines are used as "section
>> +		 * names" identifying the type of workarounds that
>> +		 * follow (e.g. "GT Workarounds", "Engine Workarounds"
>> +		 * etc).
>> +		 */
>> +		if (*a++ != '\t')
>> +			goto next_line;
>> +
>> +		while (*a == *b && !(*a == '\0' || *a == '\n' || *b == '\0')) {
>> +			a++;
>> +			b++;
>> +		}
>> +
>> +		if ((*a == '\0' || *a == '\n') && *b == '\0')
>> +			return true;
>> +
>> +	next_line:
>> +		/* No match for this line, advance to the next one. */
>> +		while (*a != '\n' && *a != '\0')
>> +			a++;
>> +
>> +		if (*a == '\n')
>> +			a++;
>> +	}
>> +
>> +	return false;
>> +}
>> +
>>  static int debugfs_file_has_wa(int drm_fd, int debugfs_fd,
>>  			       const char *debugfs_name, const char *wa)
>>  {
>> @@ -23,7 +60,7 @@ static int debugfs_file_has_wa(int drm_fd, int debugfs_fd,
>>  
>>  	debugfs_dump = igt_sysfs_get(debugfs_fd, debugfs_name);
>>  	if (debugfs_dump) {
>> -		char *has_wa = strstr(debugfs_dump, wa);
>> +		bool has_wa = debugfs_dump_has_wa(debugfs_dump, wa);
>>  
>>  		free(debugfs_dump);
>>  
>> 
>> -- 
>> 2.55.0
>> 
>
> -- 
> Matt Roper
> Graphics Software Engineer
> Linux GPU Platform Enablement
> Intel Corporation