[PATCH i-g-t v4 4/4] lib/igt_sysfs: make sure to write empty strings

Lucas De Marchi lucas.demarchi at intel.com
Wed Feb 28 22:31:34 UTC 2024


echo -n "" > /sys/module/<modulename>/parameters/<param>

doesn't really work as it will just create a open() + close() expecting
the file to be truncated. The same issue happens with igt as it will
stop writing when there are 0 chars to write. Special case the empty
string so it always write a '\0' and make sure igt_sysfs_set() accounts
for the extra null char.

Shell example:
	# echo -n "/foo" > /sys/module/firmware_class/parameters/path
	# cat /sys/module/firmware_class/parameters/path
	/foo
	# echo -n "" > /sys/module/firmware_class/parameters/path
	/foo
	# # make sure to actually write a \0:
	echo -ne "\0" > /sys/module/firmware_class/parameters/path
	# cat /sys/module/firmware_class/parameters/path

Same thing happens when testing igt_sysfs_set():
       int dirfd = open("/sys/module/firmware_class/parameters", O_RDONLY);
       igt_sysfs_set(dirfd, "path", "");

Previously it was not really setting the param.

v2:
  - Fix return code from igt_sysfs_vprintf() to differentiate between
    writing 1 or 0 chars (Janusz)
  - Document the behavior of igt_sysfs_set() as being a higher-level
    than igt_sysfs_write().
v3:
  - Partial "back to v1": include the \0 in the write only when writing
    an empty string: there are some files in sysfs like pm_test that
    don't like
v4:
  - Fix return code in igt_sysfs_vprintf() when fmt string is something
    like "%c<more_fmt>", c, ..." so it still returns > 0 (Janusz)

Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
---
 lib/igt_sysfs.c | 41 +++++++++++++++++++++++++++++++++--------
 1 file changed, 33 insertions(+), 8 deletions(-)

diff --git a/lib/igt_sysfs.c b/lib/igt_sysfs.c
index a1ff5655d..0c5817eb1 100644
--- a/lib/igt_sysfs.c
+++ b/lib/igt_sysfs.c
@@ -348,7 +348,10 @@ int igt_sysfs_get_num_gt(int device)
  * @data: the block to write from
  * @len: the length to write
  *
- * This writes @len bytes from @data to the sysfs file.
+ * This writes @len bytes from @data to the sysfs file.  Contrary to
+ * igt_sysfs_set(), this does not automatically write a null char if len is 0.
+ * It's caller responsibility to pass the right len according to the data being
+ * written.
  *
  * Returns:
  * The number of bytes written, or -errno on error.
@@ -407,6 +410,14 @@ int igt_sysfs_read(int dir, const char *attr, void *data, int len)
 bool igt_sysfs_set(int dir, const char *attr, const char *value)
 {
 	int len = strlen(value);
+
+	/*
+	 * Always write at least 1 char, the null byte, otherwise it
+	 * won't write anything on sysfs.
+	 */
+	if (!len)
+		len = 1;
+
 	return igt_sysfs_write(dir, attr, value, len) == len;
 }
 
@@ -506,7 +517,7 @@ int igt_sysfs_vprintf(int dir, const char *attr, const char *fmt, va_list ap)
 {
 	char stack[128], *buf = stack;
 	va_list tmp;
-	int ret, fd;
+	int ret, fd, len;
 
 	fd = openat(dir, attr, O_WRONLY);
 	if (igt_debug_on(fd < 0))
@@ -520,17 +531,27 @@ int igt_sysfs_vprintf(int dir, const char *attr, const char *fmt, va_list ap)
 		goto end;
 	}
 
-	if (ret > sizeof(stack)) {
-		unsigned int len = ret + 1;
-
-		buf = malloc(len);
+	len = ret;
+
+	if (!ret) {
+		/*
+		 * Make sure to always issue a write() syscall, even if writing
+		 * an empty string, otherwise values in sysfs like module
+		 * parameters don't really get overwritten.  vsnprintf()
+		 * guarantees to return a \0 terminated string, so just add
+		 * that char. The return code is still the same as before, to
+		 * abstract that from caller.
+		 */
+		ret = 1;
+	} else if (ret > sizeof(stack)) {
+		buf = malloc(len + 1);
 		if (igt_debug_on(!buf)) {
 			ret = -ENOMEM;
 			goto end;
 		}
 
-		ret = vsnprintf(buf, len, fmt, ap);
-		if (igt_debug_on(ret != len - 1)) {
+		ret = vsnprintf(buf, len + 1, fmt, ap);
+		if (igt_debug_on(ret != len)) {
 			ret = -EINVAL;
 			goto free_buf;
 		}
@@ -538,6 +559,10 @@ int igt_sysfs_vprintf(int dir, const char *attr, const char *fmt, va_list ap)
 
 	ret = igt_writen(fd, buf, ret);
 
+	/* Caller shouldn't know about special sysfs handling, just return 0 */
+	if (!len && ret == 1)
+		ret = 0;
+
 free_buf:
 	if (buf != stack)
 		free(buf);
-- 
2.43.0



More information about the igt-dev mailing list