Re: [PATCHv2] evdev: fix evdev_write return value on partial writes

Hi Peter,

On Thu, Jan 27, 2011 at 12:21:30PM +0100, Peter Korsgaard wrote:

quoted

quoted

quoted

quoted

quoted

"Henrik" == Henrik Rydberg [off-list ref] writes:

 >> @@ -321,6 +321,9 @@ static ssize_t evdev_write(struct file *file, const 
 >> char __user *buffer,
 >> struct input_event event;
 >> int retval;
 >> 
 >> +	if (count < input_event_size())
 >> +		return -EINVAL;
 >> +

 Henrik> This assumes that write will only ever be called with sufficient
 Henrik> data. It is not an error to write (and report) less data than
 Henrik> specified, so perhaps the above will yield unpleasant surprises.

Well, like this it's consistent with evdev_read(). We can only consume
complete input_event structures, so we can either return 0 (no events
consumed) or -EINVAL (invalid data).

Before the patch we returned sizeof input_event (if data after write
buffer is accessible) or -EINVAL otherwise.

The v1 patch returned 0, but that causes problems with userspace, as it
often does:

    wlile (len) {
        n = write(fd, buf, len);
        if (n <= 0) break;
        len -= n;
        buf += n;
    }

Which will then loop forever.

Well this code won't loop forever, since n == 0. Only if you do:

    if (n < 0) break;

That is.

baruch

Returning -EINVAL on something that can never work seems like the sanest
option.

-- 
                                                     ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch@tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -