Re: [PATCH 2/2] Input: Add EVIOC mechanism for MT slots

["Henrik Rydberg" <rydberg@euromail.se>]

Hi Dmitry,

> > > +static int evdev_handle_mt_request(struct input_dev *dev,
> > > +				   unsigned int size,
> > > +				   int __user *ip)
> > > +{
> > > +	const struct input_mt_slot *mt = dev->mt;
> > > +	unsigned int code;
> > > +	int i;
> > > +
> > > +	if (get_user(code, &ip[0]))
> > > +		return -EFAULT;
> > > +	if (!input_is_mt_value(code))
> > > +		return -EINVAL;
> > > +
> > > +	if (size < sizeof(struct input_mt_request) + dev->mtsize * sizeof(int))
> > > +		return -EINVAL;
> > 
> > Why not fill in up to the size of the array provided?
> 
> Yes, I think we do the same with other ioctls as well.

Agreed, will do it that way.

> > > +/**
> > > + * struct input_mt_request - used by EVIOCGMTSLOTS ioctl
> > > + * @code: ABS_MT code to read
> > > + * @num_values: number of values written to the value array
> > > + * @values: value array, one value per slot
> > > + *
> > > + * The structure is used to retrieve MT slot data. Before the call,
> > > + * code is set to the wanted ABS_MT event type. On return, the value
> > > + * array is filled with the slot values for the specified ABS_MT code,
> > > + * and num_values is set to the actual number of slots.
> > > + *
> > > + * The call argument (len) is the size of the return buffer, satisfying
> > > + *
> > > + * len >= sizeof(input_mt_request) + sizeof(__s32) * number_of_slots
> > > + *
> > > + * If the request code is not an ABS_MT value, or if len is too small,
> > > + * -EINVAL is returned.
> > > + */
> > > +struct input_mt_request {
> > > +	__u32 code;
> > > +	__u32 num_values;
> 
> Hmm, might avoid this one... Userspace can do EVIOCGABS to get number of
> slots and then it will know how much data EVIOCGMTSLOTS will return...

I think it might be good to keep the length, to avoid possible future
synchronization problems. Perhaps we could get rid of the structure
though, for instance by specifying the write as a 32-bit code and the
read buffer as length, value, value...

Thanks,
Henrik