Re: [PATCH v2] lib/intel_wa: Assert on error instead of returning -1

["Dixit, Ashutosh" <ashutosh.dixit@intel.com>]

On Wed, 13 May 2026 23:36:55 -0700, Shekhar Chauhan wrote:
>
> igt_has_intel_wa() returns 0, 1, or -1, but there is no point
> distinguishing the error and no-workaround cases for callers. Replace
> the error return with igt_assert() and simplify debugfs_file_has_wa() by
> dropping the igt_debugfs_exists() check, since igt_sysfs_get() returns
> NULL for missing files anyway.
>
> v2: Make the func as bool instead of int.

LGTM now:

Reviewed-by: Ashutosh Dixit <ashutosh.dixit@intel.com>

>
> Signed-off-by: Shekhar Chauhan <shekhar.chauhan@intel.com>
> ---
>  lib/intel_wa.c | 19 +++++++++----------
>  lib/intel_wa.h |  2 +-
>  2 files changed, 10 insertions(+), 11 deletions(-)
>
> diff --git a/lib/intel_wa.c b/lib/intel_wa.c
> index 727dd6c98..e4fe7b0ee 100644
> --- a/lib/intel_wa.c
> +++ b/lib/intel_wa.c
> @@ -7,18 +7,18 @@
>  #include <stdint.h>
>  #include <stdio.h>
>
> +#include "igt_core.h"
>  #include "igt_debugfs.h"
>  #include "igt_sysfs.h"
>  #include "intel_wa.h"
>  #include "xe/xe_query.h"
>
> -static int debugfs_file_has_wa(int drm_fd, int debugfs_fd,
> +static bool debugfs_file_has_wa(int drm_fd, int debugfs_fd,
>			       const char *debugfs_name, const char *wa)
>  {
>	char *debugfs_dump;
>
> -	if (!igt_debugfs_exists(drm_fd, debugfs_name, O_RDONLY))
> -		return -1;
> +	igt_assert(igt_debugfs_exists(drm_fd, debugfs_name, O_RDONLY));
>
>	debugfs_dump = igt_sysfs_get(debugfs_fd, debugfs_name);
>	if (debugfs_dump) {
> @@ -27,10 +27,10 @@ static int debugfs_file_has_wa(int drm_fd, int debugfs_fd,
>		free(debugfs_dump);
>
>		if (has_wa)
> -			return 1;
> +			return true;
>	}
>
> -	return 0;
> +	return false;
>  }
>
>  /**
> @@ -38,18 +38,17 @@ static int debugfs_file_has_wa(int drm_fd, int debugfs_fd,
>   * @drm_fd:	A drm file descriptor
>   * @check_wa:	Workaround to be checked
>   *
> - * Returns:	0 if no WA, 1 if WA present, -1 on error
> + * Returns:	true if WA present, false otherwise
>   */
> -int igt_has_intel_wa(int drm_fd, const char *check_wa)
> +bool igt_has_intel_wa(int drm_fd, const char *check_wa)
>  {
> -	int ret = 0;
> +	bool ret = 0;
>	int debugfs_fd;
>	unsigned int xe;
>	char name[256];
>
>	debugfs_fd = igt_debugfs_dir(drm_fd);
> -	if (debugfs_fd == -1)
> -		return -1;
> +	igt_assert(debugfs_fd >= 0);
>
>	xe_for_each_gt(drm_fd, xe) {
>		sprintf(name, "gt%d/workarounds", xe);
> diff --git a/lib/intel_wa.h b/lib/intel_wa.h
> index 765a5948e..34cafecc4 100644
> --- a/lib/intel_wa.h
> +++ b/lib/intel_wa.h
> @@ -6,6 +6,6 @@
>  #ifndef __INTEL_WA_H__
>  #define __INTEL_WA_H__
>
> -int igt_has_intel_wa(int drm_fd, const char *check_wa);
> +bool igt_has_intel_wa(int drm_fd, const char *check_wa);
>
>  #endif /* __INTEL_WA_H__ */
> --
> 2.53.0
>