[PATCH i-g-t v2] lib/igt_sysfs: make sure to write empty strings
Kamil Konieczny
kamil.konieczny at linux.intel.com
Thu Feb 1 13:12:04 UTC 2024
Hi Lucas,
On 2024-01-31 at 19:11:56 -0800, Lucas De Marchi wrote:
> 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
> /foo
>
> 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.
>
> Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
> ---
> lib/igt_sysfs.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/lib/igt_sysfs.c b/lib/igt_sysfs.c
> index 567b4f6d5..64c2aaabb 100644
> --- a/lib/igt_sysfs.c
> +++ b/lib/igt_sysfs.c
> @@ -406,7 +406,7 @@ 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);
> + int len = strlen(value) + 1;
> return igt_sysfs_write(dir, attr, value, len) == len;
Why not adding write "\n" at end?
> }
>
> @@ -513,8 +513,16 @@ int igt_sysfs_vprintf(int dir, const char *attr, const char *fmt, va_list ap)
> return -errno;
>
> va_copy(tmp, ap);
> - ret = vsnprintf(buf, sizeof(stack), fmt, tmp);
> + ret = vsnprintf(stack, sizeof(stack), fmt, tmp);
------------------- ^^^^^
Nice catch and looks like a separate bug fix, please send
as separate patch.
Regards,
Kamil
> va_end(tmp);
> +
> + /*
> + * 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
> + */
> + if (igt_debug_on(ret == 0))
> + return igt_writen(fd, "", 1);
> +
> if (igt_debug_on(ret < 0))
> return -EINVAL;
>
> --
> 2.43.0
>
More information about the igt-dev
mailing list