Hi,
> > + spin_lock_irqsave(&vi->lock, flags);
> > + while ((event = virtqueue_get_buf(vi->evt, &len)) != NULL) {
> > + input_event(vi->idev,
> > + le16_to_cpu(event->type),
> > + le16_to_cpu(event->code),
> > + le32_to_cpu(event->value));
>
> What happens if input layer gets an
> unexpected event code or value?
input layer checks it and ignores events not supported (according to the
support bitmaps).
> > +static int virtinput_send_status(struct virtio_input *vi,
> > + u16 type, u16 code, s32 value)
> > +{
> This means that caller will get errors if it happens to call
> send_status at a rate that's faster than host's consumption of them.
> To me this looks wrong.
> Poking at input layer, it seems to simply discard errors.
> Is it always safe to discard status updates?
Typical use case is updating the leds of your keyboard. Loosing one
update isn't the end of the world. Also that are _very_ low rate
events, and in case that really actually happens you likely have bigger
problems anyway ;)
> > +static void virtinput_cfg_bits(struct virtio_input *vi, int select, int
> > subsel,
> > + unsigned long *bits, unsigned int bitcount)
> > +{
> > + unsigned int bit;
> > + size_t bytes;
> > + u8 *virtio_bits;
> > +
> > + bytes = virtinput_cfg_select(vi, select, subsel);
> > + if (!bytes)
> > + return;
>
> How about limiting bytes to sizeof struct virtio_input_config->u?
It's limited to 256 anyway because size is u8 in config space.
> > + size = virtinput_cfg_select(vi, VIRTIO_INPUT_CFG_ID_DEVIDS, 0);
> > + if (size >= 8) {
>
> What does 8 mean here? Should be sizeof virtio_input_devids?
Yes, fixed.
> > +struct virtio_input_config {
> > + __u8 select;
> > + __u8 subsel;
> > + __u8 size;
> > + __u8 reserved;
> > + union {
> > + char string[128];
> > + __u8 bitmap[128];
>
> I note that neither string nor bitmap are used by
> driver. What are they in aid of?
Fixed. Just sloppy coding, the name should use u.string, the bitmap
u.bitmap, instead of just "u" (although for offsetof it doesn't make a
difference).
cheers,
Gerd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
