On 16/09/24(Mon) 23:12, Ulf Brosziewski wrote:
> This patch fixes a bug in the 'mousecfg' part of wsconsctl, which can
> lead to some display errors if wsconsctl commands are constructed in
> a somewhat unusual way.  When wsconsctl fields that are linked to more
> than one wsmouse parameter are being changed and subsequently read
> within a single command, stale values are read from the mousecfg buffer:
> 
> $ wsconsctl mouse.tp.edges
> mouse.tp.edges=0.0,0.5,0.0,0.5
> $ wsconsctl mouse.tp.edges=1,2,3,4 mouse.tp.edges
> mouse.tp.edges -> 1.0,2.0,3.0,4.0
> mouse.tp.edges=0.0,0.5,0.0,4.0
> $ wsconsctl mouse.tp.edges
> mouse.tp.edges=1.0,2.0,3.0,4.0
> 
> Obviously, there is no reason to chain WRITE and READ operations in
> this way, but it's possible.

Shouldn't we use the same idiom present in the other loops?

	if ((n = index_of(field->params[i].key)) >= 0)
		cfg_buffer[n].value = field->params[i].value;

Or are we sure the values cannot be changed to something invalid between
the two ioctls?

> Index: mousecfg.c
> ===================================================================
> RCS file: /cvs/src/sbin/wsconsctl/mousecfg.c,v
> retrieving revision 1.10
> diff -u -p -r1.10 mousecfg.c
> --- mousecfg.c	2 Jul 2023 21:44:04 -0000	1.10
> +++ mousecfg.c	16 Sep 2024 20:16:56 -0000
> @@ -222,8 +222,10 @@ mousecfg_put_field(int fd, struct wsmous
>  	    || (err = ioctl(fd, WSMOUSEIO_GETPARAMS, field)))
>  		return err;
> 
> -	for (i = 0; i < field->nparams; i++)
> +	for (i = 0; i < field->nparams; i++) {
> +		n = index_of(field->params[i].key);
>  		cfg_buffer[n].value = field->params[i].value;
> +	}
> 
>  	return (0);
>  }
>