On 9/21/24 13:16, Martin Pieuchot wrote:
> 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?

In its ioctl operations wsmouse(4) does not and must not change the 'key'
fields, it would break the mechanism.  Invalid 'key' entries may only
result from bad input in a 'mouse.param' argument.  However, the check
is necessary everywhere else, we could add it here for stylistic reasons.

> 
>> 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);
>>  }
>>
>