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

[Kamil Konieczny]

Hi,

On 2024-02-01 at 14:12:08 +0100, Kamil Konieczny wrote:
> 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?

So what about changing this only for zero?

if (!len)
        ++len;

+cc Janusz

Regards,
Kamil

> 
> >  }
> >  
> > @@ -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
> >