[PATCH i-g-t v3 3/6] tests/intel/xe_fault_injection: Add helper to start injecting fault after x iterations

[Lucas De Marchi]

On Thu, Mar 06, 2025 at 03:40:38PM +0530, Satyanarayana K V P wrote:
>As per the current implementation, the fault is injected on the first call
>of the error injection function. Introduce an environment variable
>IGT_FAULT_INJECT_ITERATION using which, an error can be injected at
>specific function call.
>
>Signed-off-by: Satyanarayana K V P <satyanarayana.k.v.p at intel.com>
>Cc: Michał Wajdeczko <michal.wajdeczko at intel.com>
>Cc: Francois Dugast <francois.dugast at intel.com>
>---
> tests/intel/xe_fault_injection.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
>diff --git a/tests/intel/xe_fault_injection.c b/tests/intel/xe_fault_injection.c
>index 019ddfb7f..1bf8ef1d8 100644
>--- a/tests/intel/xe_fault_injection.c
>+++ b/tests/intel/xe_fault_injection.c
>@@ -42,6 +42,18 @@ struct fault_injection_params {
> 	uint32_t space;
> };
>
>+/**
>+ *  Introduce a new environmental variable IGT_FAULT_INJECT_ITERATION
>+ *  using which an error can be injected at specific function call.
>+ */
>+static int get_fault_inject_iter(void)

a static function that is not used? that would create build warnings.
Please squash it in the patch that actually uses it.


>+{
>+	const char *env = getenv("IGT_FAULT_INJECT_ITERATION");
>+
>+	/* Return 0 if not exported / -ve value */
>+	return env ? (atoi(env) > 0 ? atoi(env) : 0) : 0;

this is abusing the `?` operator. This is much more legible:

if (!env || atoi(env) <= 0)
	return 0;

return atoi(env);

or even save it in a ret variable.

what I really miss is xe_fault_injection as a tool rather than a test.
In that case a command line switch or env var would make perfect sense.
As a test it seems weird to read an env var.

Lucas De Marchi

>+}
>+
> static int fail_function_open(void)
> {
> 	int debugfs_fail_function_dir_fd;
>-- 
>2.35.3
>