[PATCH i-g-t 1/1] tests/intel/xe_sysfs_scheduler: Add invalid string test for engine sysfs properties

[PATCH i-g-t 1/1] tests/intel/xe_sysfs_scheduler: Add invalid string test for engine sysfs properties

[replace_with Sobin Thomas]

From: Sobin Thomas <sobin.thomas@intel.com>

This test validates that invalid string inputs are correctly rejected
by engine sysfs write. It ensures that the property values remain
unchanged when invalid inputs are provided.

v2: Added error check for return values for igt_sysfs_scanf and
igt_sysfs_printf. Removed the changes for fault injection in
tests/intel/xe_fault_injection.c

v1: Initial changes for checking error string.

Signed-off-by: Sobin Thomas <sobin.thomas@intel.com>
---
 tests/intel/xe_sysfs_scheduler.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/tests/intel/xe_sysfs_scheduler.c b/tests/intel/xe_sysfs_scheduler.c
index 4fc764f82..594c51d44 100644
--- a/tests/intel/xe_sysfs_scheduler.c
+++ b/tests/intel/xe_sysfs_scheduler.c
@@ -10,6 +10,11 @@
  * Sub-category: SysMan tests
  * Functionality: scheduler control interface
  *
+ * SUBTEST: %s-invalid-string
+ * Description: Test to check if %s arg[1] schedule parameter checks for
+ * min max values.
+ * Test category: Negative string test
+ *
  * SUBTEST: %s-invalid
  * Description: Test to check if %s arg[1] schedule parameter rejects any
  *		unrepresentable intervals.
@@ -121,6 +126,29 @@ static void test_min_max(int xe, int engine, const char **property,
 	igt_assert_eq(set, default_max);
 }
 
+static void test_invalid_string(int xe, int engine, const char **property,
+				uint16_t class, int gt)
+{
+	unsigned int saved, set;
+	char invalid_input[] = "999abc";
+
+	for (int i = 0; i < 3; i++) {
+		igt_assert(igt_sysfs_scanf(engine, property[i], "%u", &saved) == 1);
+		igt_info("Initial %s: %u\n", property[i], saved);
+		if (igt_sysfs_printf(engine, property[i], "%s", invalid_input) > 0) {
+			igt_critical("invalid value %s can be set to property %s\n",
+					invalid_input, property[i]);
+			continue;
+		}
+
+		igt_assert(igt_sysfs_scanf(engine, property[i], "%u", &set) == 0);
+
+		//Check if the values are unchanged.
+		igt_assert_eq(set, saved);
+	}
+}
+
+
 #define MAX_GTS 8
 igt_main
 {
@@ -130,6 +158,7 @@ igt_main
 	} tests[] = {
 		{ "invalid", test_invalid },
 		{ "min-max", test_min_max },
+		{ "invalid-string", test_invalid_string },
 		{ }
 	};
 
-- 
2.34.1

RE: [PATCH i-g-t 1/1] tests/intel/xe_sysfs_scheduler: Add invalid string test for engine sysfs properties

[Piatkowski, Dominik Karol]

Hi Sobin,

For future submissions, please don't change cover letter's patch number from 0 to not confuse patchwork.

> -----Original Message-----
> From: Thomas, Sobin <sobin.thomas@intel.com>
> Sent: Monday, March 31, 2025 4:28 PM
> To: igt-dev@lists.freedesktop.org
> Cc: Konieczny, Kamil <kamil.konieczny@intel.com>; Piatkowski, Dominik Karol
> <dominik.karol.piatkowski@intel.com>
> Subject: [PATCH i-g-t 1/1] tests/intel/xe_sysfs_scheduler: Add invalid string

Missing v2 after PATCH.

> test for engine sysfs properties
> 
> From: Sobin Thomas <sobin.thomas@intel.com>
> 
> This test validates that invalid string inputs are correctly rejected
> by engine sysfs write. It ensures that the property values remain
> unchanged when invalid inputs are provided.
> 
> v2: Added error check for return values for igt_sysfs_scanf and
> igt_sysfs_printf. Removed the changes for fault injection in
> tests/intel/xe_fault_injection.c
> 
> v1: Initial changes for checking error string.
> 
> Signed-off-by: Sobin Thomas <sobin.thomas@intel.com>
> ---
>  tests/intel/xe_sysfs_scheduler.c | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/tests/intel/xe_sysfs_scheduler.c b/tests/intel/xe_sysfs_scheduler.c
> index 4fc764f82..594c51d44 100644
> --- a/tests/intel/xe_sysfs_scheduler.c
> +++ b/tests/intel/xe_sysfs_scheduler.c
> @@ -10,6 +10,11 @@
>   * Sub-category: SysMan tests
>   * Functionality: scheduler control interface
>   *
> + * SUBTEST: %s-invalid-string
> + * Description: Test to check if %s arg[1] schedule parameter checks for
> + * min max values.

Please align this line in a similar way to other subtests in this file.

> + * Test category: Negative string test
> + *
>   * SUBTEST: %s-invalid
>   * Description: Test to check if %s arg[1] schedule parameter rejects any
>   *		unrepresentable intervals.
> @@ -121,6 +126,29 @@ static void test_min_max(int xe, int engine, const
> char **property,
>  	igt_assert_eq(set, default_max);
>  }
> 
> +static void test_invalid_string(int xe, int engine, const char **property,
> +				uint16_t class, int gt)
> +{
> +	unsigned int saved, set;
> +	char invalid_input[] = "999abc";
> +
> +	for (int i = 0; i < 3; i++) {
> +		igt_assert(igt_sysfs_scanf(engine, property[i], "%u", &saved)
> == 1);
> +		igt_info("Initial %s: %u\n", property[i], saved);
> +		if (igt_sysfs_printf(engine, property[i], "%s", invalid_input) >
> 0) {
> +			igt_critical("invalid value %s can be set to property
> %s\n",
> +					invalid_input, property[i]);
> +			continue;

I see a flaw in the test flow. In case of igt_sysfs_printf not failing as expected, we print message with IGT_LOG_CRITICAL level and just continue the loop, not hitting asserts below. The thing is, the igt_critical does not make the test fail, despite the fact that we found a bug. Please fix it.

Thanks,
Dominik Karol

> +		}
> +
> +		igt_assert(igt_sysfs_scanf(engine, property[i], "%u", &set) ==
> 0);
> +
> +		//Check if the values are unchanged.
> +		igt_assert_eq(set, saved);
> +	}
> +}
> +
> +
>  #define MAX_GTS 8
>  igt_main
>  {
> @@ -130,6 +158,7 @@ igt_main
>  	} tests[] = {
>  		{ "invalid", test_invalid },
>  		{ "min-max", test_min_max },
> +		{ "invalid-string", test_invalid_string },
>  		{ }
>  	};
> 
> --
> 2.34.1

Re: [PATCH i-g-t 1/1] tests/intel/xe_sysfs_scheduler: Add invalid string test for engine sysfs properties

[Kamil Konieczny]

Hi replace_with,
On 2025-03-31 at 14:27:49 +0000, replace_with Sobin Thomas wrote:
> From: Sobin Thomas <sobin.thomas@intel.com>

one nit about configuring your mailer, please remove 'replace_with'
from it, now in a header there is From: with:

replace_with Sobin Thomas <sobin.thomas@intel.com>

and this should be just:

Sobin Thomas <sobin.thomas@intel.com>

Few more nits in addition to what Dominik Karol already wrote.

> 
> This test validates that invalid string inputs are correctly rejected
> by engine sysfs write. It ensures that the property values remain
> unchanged when invalid inputs are provided.
> 
> v2: Added error check for return values for igt_sysfs_scanf and
> igt_sysfs_printf. Removed the changes for fault injection in
> tests/intel/xe_fault_injection.c
> 
> v1: Initial changes for checking error string.
> 
> Signed-off-by: Sobin Thomas <sobin.thomas@intel.com>
> ---
>  tests/intel/xe_sysfs_scheduler.c | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/tests/intel/xe_sysfs_scheduler.c b/tests/intel/xe_sysfs_scheduler.c
> index 4fc764f82..594c51d44 100644
> --- a/tests/intel/xe_sysfs_scheduler.c
> +++ b/tests/intel/xe_sysfs_scheduler.c
> @@ -10,6 +10,11 @@
>   * Sub-category: SysMan tests
>   * Functionality: scheduler control interface
>   *
> + * SUBTEST: %s-invalid-string
> + * Description: Test to check if %s arg[1] schedule parameter checks for
> + * min max values.
> + * Test category: Negative string test
> + *
>   * SUBTEST: %s-invalid
>   * Description: Test to check if %s arg[1] schedule parameter rejects any
>   *		unrepresentable intervals.
> @@ -121,6 +126,29 @@ static void test_min_max(int xe, int engine, const char **property,
>  	igt_assert_eq(set, default_max);
>  }
>  
> +static void test_invalid_string(int xe, int engine, const char **property,
> +				uint16_t class, int gt)
> +{
> +	unsigned int saved, set;
> +	char invalid_input[] = "999abc";

If we start to test it, why not allocate more than one page here
and fill it with gargabe from rand() ? I mean letters from a....zA...Z
That big overflow attempt could be a separate subtest, also with all
letters and/or digits.

> +
> +	for (int i = 0; i < 3; i++) {
> +		igt_assert(igt_sysfs_scanf(engine, property[i], "%u", &saved) == 1);
> +		igt_info("Initial %s: %u\n", property[i], saved);
> +		if (igt_sysfs_printf(engine, property[i], "%s", invalid_input) > 0) {
> +			igt_critical("invalid value %s can be set to property %s\n",
> +					invalid_input, property[i]);
> +			continue;

Better to assert here, did you run this on real hardware?
Please test it yourself with igt_runner and as standalone run with

sudo ./xe_sysfs_scheduler.c --r invalid-string

> +		}
> +
> +		igt_assert(igt_sysfs_scanf(engine, property[i], "%u", &set) == 0);
> +
> +		//Check if the values are unchanged.

Please use rather C-style comments or remove this line:
		/* Check if the values are unchanged */

> +		igt_assert_eq(set, saved);
> +	}
> +}
> +
> +
>  #define MAX_GTS 8
>  igt_main
>  {
> @@ -130,6 +158,7 @@ igt_main
>  	} tests[] = {
>  		{ "invalid", test_invalid },
>  		{ "min-max", test_min_max },
> +		{ "invalid-string", test_invalid_string },

This should be in alphabetical order, so before 'min-max'.

Regards,
Kamil

>  		{ }
>  	};
>  
> -- 
> 2.34.1
> 

[PATCH v4 i-g-t 0/1] tests/intel/xe_sysfs_scheduler: Add invalid string test for engine sysfs properties

[Sobin Thomas]

This test validates that invalid string inputs are correctly rejected
by engine sysfs write. It ensures that the property values remain
unchanged when invalid inputs with large strings are provided.

Sobin Thomas (1):
  tests/intel/xe_sysfs_scheduler: Add invalid string test for engine
    sysfs properties

 tests/intel/xe_sysfs_scheduler.c | 98 ++++++++++++++++++++++++++++++++
 1 file changed, 98 insertions(+)

Signed-off-by: Sobin Thomas <sobin.thomas@intel.com>
-- 
2.34.1

[PATCH i-g-t 1/1] tests/intel/xe_sysfs_scheduler: Add invalid string test for engine sysfs properties

[Sobin Thomas]

This test validates that invalid string inputs are correctly rejected
by engine sysfs write. It ensures that the property values remain
unchanged when invalid inputs are provided.

v4: Added subtests to test large (4k) random strings (kamil).
Replaced igt macro.

v3: Fixed the test logic. Fixed the tabs

v2: Added error check for return values for igt_sysfs_scanf and
igt_sysfs_printf. Removed the changes for fault injection in
tests/intel/xe_fault_injection.c

v1: Initial changes for checking error string.

Signed-off-by: Sobin Thomas <sobin.thomas@intel.com>
---
 tests/intel/xe_sysfs_scheduler.c | 98 ++++++++++++++++++++++++++++++++
 1 file changed, 98 insertions(+)

diff --git a/tests/intel/xe_sysfs_scheduler.c b/tests/intel/xe_sysfs_scheduler.c
index 4fc764f82..285689778 100644
--- a/tests/intel/xe_sysfs_scheduler.c
+++ b/tests/intel/xe_sysfs_scheduler.c
@@ -15,6 +15,16 @@
  *		unrepresentable intervals.
  * Test category: negative test
  *
+ * SUBTEST: %s-invalid-string
+ * Description: Test to check if %s arg[1] schedule parameter checks for
+ *		invalid string values.
+ * Test category: negative test
+ *
+ * SUBTEST: %s-invalid-large-string
+ * Description: Test to check if %s arg[1] schedule parameter checks for
+ *		large invalid strings (4k).
+ * Test category: negative test
+ *
  * SUBTEST: %s-min-max
  * Description: Test to check if %s arg[1] schedule parameter checks for
  *		min max values.
@@ -39,6 +49,46 @@
 #include "xe_drm.h"
 #include "xe/xe_query.h"
 
+#define STR_LENGTH 4096
+
+/*
+ * generate_random_string function is supposed to generate random
+ * string with more bias on alphabets than numeric this function
+ * provides random string based on the output of rand() and can
+ * sometime provide same class of characters if the output of
+ * rand() is same.
+ */
+static void generate_random_string(char *str, size_t length)
+{
+	int type = 0;
+	int digit_count = 0;
+	int max_digits = length / 4;
+
+	for (size_t i = 0; i < length; i++) {
+		if (digit_count >= max_digts)
+			type = rand() % 2;
+		else
+			type = rand() % 3;
+
+		switch (type) {
+		case 0:
+			str[i] = 'A' + (rand() % 26);
+			break;
+		case 1:
+			str[i] = 'a' + (rand() % 26);
+			break;
+		case 2:
+			str[i] = '0' + (rand() % 10);
+			digit_count++;
+			break;
+		default:
+			str[i] = '_';
+			break;
+		}
+	}
+	str[length] = '\0';
+}
+
 static void test_invalid(int xe, int engine, const char **property,
 			 uint16_t class, int gt)
 {
@@ -121,6 +171,52 @@ static void test_min_max(int xe, int engine, const char **property,
 	igt_assert_eq(set, default_max);
 }
 
+static void test_invalid_string(int xe, int engine, const char **property,
+				 uint16_t class, int gt)
+{
+	unsigned int saved, set;
+	static const char invalid_input[] = "999abc";
+
+	for (int i = 0; i < 3; i++) {
+		igt_assert_eq(igt_sysfs_scanf(engine, property[i], "%u", &saved), 1);
+		igt_info("Initial %s: %u\n", property[i], saved);
+		/* Assert if the invalid write is returning negative error */
+		igt_assert_lt(igt_sysfs_printf(engine, property[i], "%s",
+			      invalid_input), 0);
+
+		igt_assert_eq(igt_sysfs_scanf(engine, property[i], "%u", &set), 1);
+		/* Check if the values are unchanged. */
+		igt_assert_eq(set, saved);
+	}
+}
+
+static void test_invalid_large_string(int xe, int engine, const char **property,
+				       uint16_t class, int gt)
+{
+	unsigned int saved, set;
+	char *random_str;
+
+	random_str = (char *)calloc(STR_LENGTH, sizeof(char));
+	igt_assert_f(random_str, "Memory allocation failed\n");
+
+	generate_random_string(random_str, STR_LENGTH);
+	igt_debug("Generated random string: %.10s...\n", random_str);
+
+	for (int i = 0; i < 3; i++) {
+		igt_assert_eq(igt_sysfs_scanf(engine, property[i], "%u", &saved), 1);
+		igt_info("Initial %s: %u\n", property[i], saved);
+
+		/* Assert if the invalid write is returning negative error */
+		igt_assert_lt(igt_sysfs_printf(engine, property[i], "%s",
+				random_str), 0);
+
+		igt_assert_eq(igt_sysfs_scanf(engine, property[i], "%u", &set), 1);
+		/* Check if the values are unchanged. */
+		igt_assert_eq(set, saved);
+	}
+	free(random_str);
+}
+
 #define MAX_GTS 8
 igt_main
 {
@@ -129,6 +225,8 @@ igt_main
 		void (*fn)(int, int, const char **, uint16_t, int);
 	} tests[] = {
 		{ "invalid", test_invalid },
+		{ "invalid-string", test_invalid_string },
+		{ "invalid-large-string", test_invalid_large_string },
 		{ "min-max", test_min_max },
 		{ }
 	};
-- 
2.34.1

RE: [PATCH i-g-t 1/1] tests/intel/xe_sysfs_scheduler: Add invalid string test for engine sysfs properties

[Piatkowski, Dominik Karol]

Hi Sobin,

> -----Original Message-----
> From: Thomas, Sobin <sobin.thomas@intel.com>
> Sent: Tuesday, April 8, 2025 6:19 AM
> To: igt-dev@lists.freedesktop.org
> Cc: Piatkowski, Dominik Karol <dominik.karol.piatkowski@intel.com>;
> Konieczny, Kamil <kamil.konieczny@intel.com>
> Subject: [PATCH i-g-t 1/1] tests/intel/xe_sysfs_scheduler: Add invalid string
> test for engine sysfs properties

Please remember to add version number to patch subject.

> 
> This test validates that invalid string inputs are correctly rejected
> by engine sysfs write. It ensures that the property values remain
> unchanged when invalid inputs are provided.
> 
> v4: Added subtests to test large (4k) random strings (kamil).
> Replaced igt macro.
> 
> v3: Fixed the test logic. Fixed the tabs
> 
> v2: Added error check for return values for igt_sysfs_scanf and
> igt_sysfs_printf. Removed the changes for fault injection in
> tests/intel/xe_fault_injection.c
> 
> v1: Initial changes for checking error string.
> 
> Signed-off-by: Sobin Thomas <sobin.thomas@intel.com>
> ---
>  tests/intel/xe_sysfs_scheduler.c | 98
> ++++++++++++++++++++++++++++++++
>  1 file changed, 98 insertions(+)
> 
> diff --git a/tests/intel/xe_sysfs_scheduler.c b/tests/intel/xe_sysfs_scheduler.c
> index 4fc764f82..285689778 100644
> --- a/tests/intel/xe_sysfs_scheduler.c
> +++ b/tests/intel/xe_sysfs_scheduler.c
> @@ -15,6 +15,16 @@
>   *		unrepresentable intervals.
>   * Test category: negative test
>   *
> + * SUBTEST: %s-invalid-string
> + * Description: Test to check if %s arg[1] schedule parameter checks for
> + *		invalid string values.
> + * Test category: negative test
> + *
> + * SUBTEST: %s-invalid-large-string
> + * Description: Test to check if %s arg[1] schedule parameter checks for
> + *		large invalid strings (4k).
> + * Test category: negative test
> + *
>   * SUBTEST: %s-min-max
>   * Description: Test to check if %s arg[1] schedule parameter checks for
>   *		min max values.
> @@ -39,6 +49,46 @@
>  #include "xe_drm.h"
>  #include "xe/xe_query.h"
> 
> +#define STR_LENGTH 4096
> +
> +/*
> + * generate_random_string function is supposed to generate random
> + * string with more bias on alphabets than numeric this function
> + * provides random string based on the output of rand() and can
> + * sometime provide same class of characters if the output of
> + * rand() is same.
> + */

Nitpick: this documentation could be improved. I propose:

/**
 * generate_random_string:
 * @str: pointer to string buffer
 * @length: length of string to generate, excluding null termination
 *
 * Generates random string that will always contain non-numerical characters.
 */

> +static void generate_random_string(char *str, size_t length)
> +{
> +	int type = 0;
> +	int digit_count = 0;
> +	int max_digits = length / 4;
> +
> +	for (size_t i = 0; i < length; i++) {
> +		if (digit_count >= max_digts)

digit_count > max_digits will never happen, but >= comparison will still work.

There is a typo as well, it should be `max_digits` instead of `max_digts`.
This causes build to fail.

> +			type = rand() % 2;
> +		else
> +			type = rand() % 3;
> +
> +		switch (type) {
> +		case 0:
> +			str[i] = 'A' + (rand() % 26);
> +			break;
> +		case 1:
> +			str[i] = 'a' + (rand() % 26);
> +			break;
> +		case 2:
> +			str[i] = '0' + (rand() % 10);
> +			digit_count++;
> +			break;
> +		default:
> +			str[i] = '_';
> +			break;
> +		}
> +	}
> +	str[length] = '\0';
> +}
> +
>  static void test_invalid(int xe, int engine, const char **property,
>  			 uint16_t class, int gt)
>  {
> @@ -121,6 +171,52 @@ static void test_min_max(int xe, int engine, const
> char **property,
>  	igt_assert_eq(set, default_max);
>  }
> 
> +static void test_invalid_string(int xe, int engine, const char **property,
> +				 uint16_t class, int gt)
> +{
> +	unsigned int saved, set;
> +	static const char invalid_input[] = "999abc";
> +
> +	for (int i = 0; i < 3; i++) {
> +		igt_assert_eq(igt_sysfs_scanf(engine, property[i], "%u",
> &saved), 1);
> +		igt_info("Initial %s: %u\n", property[i], saved);
> +		/* Assert if the invalid write is returning negative error */
> +		igt_assert_lt(igt_sysfs_printf(engine, property[i], "%s",
> +			      invalid_input), 0);
> +
> +		igt_assert_eq(igt_sysfs_scanf(engine, property[i], "%u",
> &set), 1);
> +		/* Check if the values are unchanged. */
> +		igt_assert_eq(set, saved);
> +	}
> +}
> +
> +static void test_invalid_large_string(int xe, int engine, const char **property,
> +				       uint16_t class, int gt)
> +{
> +	unsigned int saved, set;
> +	char *random_str;
> +
> +	random_str = (char *)calloc(STR_LENGTH, sizeof(char));

You can use malloc instead of calloc here, as we will fully overwrite the buffer
anyway.

Also, one very important note - you're allocating buffer of size STR_LENGTH,
but the generate_random_string function modifies random_str[STR_LENGTH] and
that is a buffer overflow. You need to allocate STR_LENGTH + 1 to accommodate
the null terminator. This needs to be fixed.

> +	igt_assert_f(random_str, "Memory allocation failed\n");
> +
> +	generate_random_string(random_str, STR_LENGTH);
> +	igt_debug("Generated random string: %.10s...\n", random_str);
> +
> +	for (int i = 0; i < 3; i++) {
> +		igt_assert_eq(igt_sysfs_scanf(engine, property[i], "%u",
> &saved), 1);
> +		igt_info("Initial %s: %u\n", property[i], saved);
> +
> +		/* Assert if the invalid write is returning negative error */
> +		igt_assert_lt(igt_sysfs_printf(engine, property[i], "%s",
> +				random_str), 0);

Please align this line.

With issues fixed,
Reviewed-by: Dominik Karol Piątkowski <dominik.karol.piatkowski@intel.com>

> +
> +		igt_assert_eq(igt_sysfs_scanf(engine, property[i], "%u",
> &set), 1);
> +		/* Check if the values are unchanged. */
> +		igt_assert_eq(set, saved);
> +	}
> +	free(random_str);
> +}
> +
>  #define MAX_GTS 8
>  igt_main
>  {
> @@ -129,6 +225,8 @@ igt_main
>  		void (*fn)(int, int, const char **, uint16_t, int);
>  	} tests[] = {
>  		{ "invalid", test_invalid },
> +		{ "invalid-string", test_invalid_string },
> +		{ "invalid-large-string", test_invalid_large_string },
>  		{ "min-max", test_min_max },
>  		{ }
>  	};
> --
> 2.34.1

[PATCH i-g-t 1/1] tests/intel/xe_sysfs_scheduler: Add invalid string test for engine sysfs properties

[Sobin Thomas]

This test validates that invalid string inputs are correctly rejected
by engine sysfs write. It ensures that the property values remain
unchanged when invalid inputs are provided.

v6: Fixed string termination logic in generate_random_string. (kamil)

v5: Fixed documentation and fixed error.

v4: Added subtests to test large (4k) random strings (kamil).
Replaced igt macro.

v3: Fixed the test logic. Fixed the tabs

v2: Added error check for return values for igt_sysfs_scanf and
igt_sysfs_printf. Removed the changes for fault injection in
tests/intel/xe_fault_injection.c

v1: Initial changes for checking error string.

Signed-off-by: Sobin Thomas <sobin.thomas@intel.com>
Reviewed-by: Dominik Karol Piątkowski <dominik.karol.piatkowski@intel.com>
Signed-off-by: Sobin Thomas <sobin.thomas@intel.com>
---
 tests/intel/xe_sysfs_scheduler.c | 99 ++++++++++++++++++++++++++++++++
 1 file changed, 99 insertions(+)

diff --git a/tests/intel/xe_sysfs_scheduler.c b/tests/intel/xe_sysfs_scheduler.c
index 4fc764f82..faafabd29 100644
--- a/tests/intel/xe_sysfs_scheduler.c
+++ b/tests/intel/xe_sysfs_scheduler.c
@@ -15,6 +15,16 @@
  *		unrepresentable intervals.
  * Test category: negative test
  *
+ * SUBTEST: %s-invalid-string
+ * Description: Test to check if %s arg[1] schedule parameter checks for
+ *		invalid string values.
+ * Test category: negative test
+ *
+ * SUBTEST: %s-invalid-large-string
+ * Description: Test to check if %s arg[1] schedule parameter checks for
+ *		large invalid strings (4k).
+ * Test category: negative test
+ *
  * SUBTEST: %s-min-max
  * Description: Test to check if %s arg[1] schedule parameter checks for
  *		min max values.
@@ -39,6 +49,47 @@
 #include "xe_drm.h"
 #include "xe/xe_query.h"
 
+#define STR_LENGTH 4096
+
+/**
+ * generate_random_string:
+ * @str: pointer to string buffer that will be having random string generated.
+ * @length: length of string to generate.
+ *
+ * Generates random string that will always contain non-numerical characters.
+ */
+static void generate_random_string(char *str, size_t length)
+{
+	int type = 0;
+	int digit_count = 0;
+	int max_digits = length / 4;
+	int loop_count = length - 1;
+
+	for (size_t i = 0; i < loop_count; i++) {
+		if (digit_count >= max_digits)
+			type = rand() % 2;
+		else
+			type = rand() % 3;
+
+		switch (type) {
+		case 0:
+			str[i] = 'A' + (rand() % 26);
+			break;
+		case 1:
+			str[i] = 'a' + (rand() % 26);
+			break;
+		case 2:
+			str[i] = '0' + (rand() % 10);
+			digit_count++;
+			break;
+		default:
+			str[i] = '_';
+			break;
+		}
+	}
+	str[length - 1] = '\0';
+}
+
 static void test_invalid(int xe, int engine, const char **property,
 			 uint16_t class, int gt)
 {
@@ -121,6 +172,52 @@ static void test_min_max(int xe, int engine, const char **property,
 	igt_assert_eq(set, default_max);
 }
 
+static void test_invalid_string(int xe, int engine, const char **property,
+				 uint16_t class, int gt)
+{
+	unsigned int saved, set;
+	static const char invalid_input[] = "999abc";
+
+	for (int i = 0; i < 3; i++) {
+		igt_assert_eq(igt_sysfs_scanf(engine, property[i], "%u", &saved), 1);
+		igt_info("Initial %s: %u\n", property[i], saved);
+		/* Assert if the invalid write is returning negative error */
+		igt_assert_lt(igt_sysfs_printf(engine, property[i], "%s",
+			      invalid_input), 0);
+
+		igt_assert_eq(igt_sysfs_scanf(engine, property[i], "%u", &set), 1);
+		/* Check if the values are unchanged. */
+		igt_assert_eq(set, saved);
+	}
+}
+
+static void test_invalid_large_string(int xe, int engine, const char **property,
+				       uint16_t class, int gt)
+{
+	unsigned int saved, set;
+	char *random_str;
+
+	random_str = (char *)malloc(sizeof(char) * (STR_LENGTH));
+	igt_assert_f(random_str, "Memory allocation failed\n");
+
+	generate_random_string(random_str, STR_LENGTH);
+	igt_debug("Generated random string: %.10s...\n", random_str);
+
+	for (int i = 0; i < 3; i++) {
+		igt_assert_eq(igt_sysfs_scanf(engine, property[i], "%u", &saved), 1);
+		igt_info("Initial %s: %u\n", property[i], saved);
+
+		/* Assert if the invalid write is returning negative error */
+		igt_assert_lt(igt_sysfs_printf(engine, property[i], "%s",
+			      random_str), 0);
+
+		igt_assert_eq(igt_sysfs_scanf(engine, property[i], "%u", &set), 1);
+		/* Check if the values are unchanged. */
+		igt_assert_eq(set, saved);
+	}
+	free(random_str);
+}
+
 #define MAX_GTS 8
 igt_main
 {
@@ -129,6 +226,8 @@ igt_main
 		void (*fn)(int, int, const char **, uint16_t, int);
 	} tests[] = {
 		{ "invalid", test_invalid },
+		{ "invalid-string", test_invalid_string },
+		{ "invalid-large-string", test_invalid_large_string },
 		{ "min-max", test_min_max },
 		{ }
 	};
-- 
2.34.1

Re: [PATCH i-g-t 1/1] tests/intel/xe_sysfs_scheduler: Add invalid string test for engine sysfs properties

[Kamil Konieczny]

Hi Sobin,
On 2025-04-10 at 09:17:15 +0000, Sobin Thomas wrote:
> This test validates that invalid string inputs are correctly rejected
> by engine sysfs write. It ensures that the property values remain
> unchanged when invalid inputs are provided.
> 
> v6: Fixed string termination logic in generate_random_string. (kamil)
> 
> v5: Fixed documentation and fixed error.
> 
> v4: Added subtests to test large (4k) random strings (kamil).
> Replaced igt macro.
> 
> v3: Fixed the test logic. Fixed the tabs
> 
> v2: Added error check for return values for igt_sysfs_scanf and
> igt_sysfs_printf. Removed the changes for fault injection in
> tests/intel/xe_fault_injection.c
> 
> v1: Initial changes for checking error string.
> 
> Signed-off-by: Sobin Thomas <sobin.thomas@intel.com>
> Reviewed-by: Dominik Karol Piątkowski <dominik.karol.piatkowski@intel.com>
> Signed-off-by: Sobin Thomas <sobin.thomas@intel.com>

This is repeated s-o-b, please do not add them twice nor more,
no need for resend as this could be corrected at merge.
Btw please do use script checkpatch.pl from Linux kernel, it
can spot such issues as in this case there are more - whitespace
errors (also could be corrected at merge with help of script).

Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

Please wait for Xe.CI.Full results and if a fail please check
them and reply with Cc to bug filing. After that it could be
merged.

Regards,
Kamil

> ---
>  tests/intel/xe_sysfs_scheduler.c | 99 ++++++++++++++++++++++++++++++++
>  1 file changed, 99 insertions(+)
> 
> diff --git a/tests/intel/xe_sysfs_scheduler.c b/tests/intel/xe_sysfs_scheduler.c
> index 4fc764f82..faafabd29 100644
> --- a/tests/intel/xe_sysfs_scheduler.c
> +++ b/tests/intel/xe_sysfs_scheduler.c
> @@ -15,6 +15,16 @@
>   *		unrepresentable intervals.
>   * Test category: negative test
>   *
> + * SUBTEST: %s-invalid-string
> + * Description: Test to check if %s arg[1] schedule parameter checks for
> + *		invalid string values.
> + * Test category: negative test
> + *
> + * SUBTEST: %s-invalid-large-string
> + * Description: Test to check if %s arg[1] schedule parameter checks for
> + *		large invalid strings (4k).
> + * Test category: negative test
> + *
>   * SUBTEST: %s-min-max
>   * Description: Test to check if %s arg[1] schedule parameter checks for
>   *		min max values.
> @@ -39,6 +49,47 @@
>  #include "xe_drm.h"
>  #include "xe/xe_query.h"
>  
> +#define STR_LENGTH 4096
> +
> +/**
> + * generate_random_string:
> + * @str: pointer to string buffer that will be having random string generated.
> + * @length: length of string to generate.
> + *
> + * Generates random string that will always contain non-numerical characters.
> + */
> +static void generate_random_string(char *str, size_t length)
> +{
> +	int type = 0;
> +	int digit_count = 0;
> +	int max_digits = length / 4;
> +	int loop_count = length - 1;
> +
> +	for (size_t i = 0; i < loop_count; i++) {
> +		if (digit_count >= max_digits)
> +			type = rand() % 2;
> +		else
> +			type = rand() % 3;
> +
> +		switch (type) {
> +		case 0:
> +			str[i] = 'A' + (rand() % 26);
> +			break;
> +		case 1:
> +			str[i] = 'a' + (rand() % 26);
> +			break;
> +		case 2:
> +			str[i] = '0' + (rand() % 10);
> +			digit_count++;
> +			break;
> +		default:
> +			str[i] = '_';
> +			break;
> +		}
> +	}
> +	str[length - 1] = '\0';
> +}
> +
>  static void test_invalid(int xe, int engine, const char **property,
>  			 uint16_t class, int gt)
>  {
> @@ -121,6 +172,52 @@ static void test_min_max(int xe, int engine, const char **property,
>  	igt_assert_eq(set, default_max);
>  }
>  
> +static void test_invalid_string(int xe, int engine, const char **property,
> +				 uint16_t class, int gt)
> +{
> +	unsigned int saved, set;
> +	static const char invalid_input[] = "999abc";
> +
> +	for (int i = 0; i < 3; i++) {
> +		igt_assert_eq(igt_sysfs_scanf(engine, property[i], "%u", &saved), 1);
> +		igt_info("Initial %s: %u\n", property[i], saved);
> +		/* Assert if the invalid write is returning negative error */
> +		igt_assert_lt(igt_sysfs_printf(engine, property[i], "%s",
> +			      invalid_input), 0);
> +
> +		igt_assert_eq(igt_sysfs_scanf(engine, property[i], "%u", &set), 1);
> +		/* Check if the values are unchanged. */
> +		igt_assert_eq(set, saved);
> +	}
> +}
> +
> +static void test_invalid_large_string(int xe, int engine, const char **property,
> +				       uint16_t class, int gt)
> +{
> +	unsigned int saved, set;
> +	char *random_str;
> +
> +	random_str = (char *)malloc(sizeof(char) * (STR_LENGTH));
> +	igt_assert_f(random_str, "Memory allocation failed\n");
> +
> +	generate_random_string(random_str, STR_LENGTH);
> +	igt_debug("Generated random string: %.10s...\n", random_str);
> +
> +	for (int i = 0; i < 3; i++) {
> +		igt_assert_eq(igt_sysfs_scanf(engine, property[i], "%u", &saved), 1);
> +		igt_info("Initial %s: %u\n", property[i], saved);
> +
> +		/* Assert if the invalid write is returning negative error */
> +		igt_assert_lt(igt_sysfs_printf(engine, property[i], "%s",
> +			      random_str), 0);
> +
> +		igt_assert_eq(igt_sysfs_scanf(engine, property[i], "%u", &set), 1);
> +		/* Check if the values are unchanged. */
> +		igt_assert_eq(set, saved);
> +	}
> +	free(random_str);
> +}
> +
>  #define MAX_GTS 8
>  igt_main
>  {
> @@ -129,6 +226,8 @@ igt_main
>  		void (*fn)(int, int, const char **, uint16_t, int);
>  	} tests[] = {
>  		{ "invalid", test_invalid },
> +		{ "invalid-string", test_invalid_string },
> +		{ "invalid-large-string", test_invalid_large_string },
>  		{ "min-max", test_min_max },
>  		{ }
>  	};
> -- 
> 2.34.1
>