[PATCH i-g-t v3 3/3] lib/igt_sysfs: make sure to write empty strings
Lucas De Marchi
lucas.demarchi at intel.com
Wed Feb 28 06:32:11 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
Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
---
lib/igt_sysfs.c | 33 +++++++++++++++++++++++++++++----
1 file changed, 29 insertions(+), 4 deletions(-)
diff --git a/lib/igt_sysfs.c b/lib/igt_sysfs.c
index 2997925e5..2d574cc17 100644
--- a/lib/igt_sysfs.c
+++ b/lib/igt_sysfs.c
@@ -349,6 +349,9 @@ int igt_sysfs_get_num_gt(int device)
* @len: the length to write
*
* This writes @len bytes from @data to the sysfs file.
+ * Contrary to igt_sysfs_set(), this does not automatically add a null char to
+ * terminate the data. 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,8 +531,18 @@ 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;
+ 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.
+ */
+ len = 1;
+ } else if (ret > sizeof(stack)) {
+ len = ret + 1;
buf = malloc(len);
if (igt_debug_on(!buf)) {
@@ -536,7 +557,11 @@ int igt_sysfs_vprintf(int dir, const char *attr, const char *fmt, va_list ap)
}
}
- ret = igt_writen(fd, buf, ret);
+ ret = igt_writen(fd, buf, len);
+
+ /* Caller shouldn't know about special sysfs handling, just return 0 */
+ if (ret == 1 && buf[0] == '\0')
+ ret = 0;
free_buf:
if (buf != stack)
--
2.43.0
More information about the igt-dev
mailing list