Re: [PATCH] spi: fix the read path in spidev

[David Brownell <david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>]

On Tuesday 01 July 2008, Sebastian Siewior wrote:
> This got broken by the recent "fix rmmod $spi_driver while spidev-user is active".
> I tested the rmmod & write path but didn't check the read path. I am sorry.
> The read logic changed and spidev_sync_read() + spidev_sync_write() do not return
> zero on success anymore but the number of bytes that has been transfered over the
> bus.
> This patch changes the logic and copy_to_user() gets called again.

Good catch ... did you observe this failure happening "in the wild"
or was this a code-inspection kind of thing?


> 
> The write path returns the number of bytes which are written to the underlying device
> what may be less than the requested size. This patch makes the same change to the read
> path or else we request a read of 20 bytes, get 10, don't call copy to user and
> report to the user that we read 10 bytes.
> 
> Signed-off-by: Sebastian Siewior <bigeasy-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>

Acked-by: David Brownell <dbrownell-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>


> Cc:  David Brownell <dbrownell-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
> ---
>  drivers/spi/spidev.c |   10 ++++------
>  1 files changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/spi/spidev.c b/drivers/spi/spidev.c
> index 72282cd..60ec6f7 100644
> --- a/drivers/spi/spidev.c
> +++ b/drivers/spi/spidev.c
> @@ -167,14 +167,14 @@ spidev_read(struct file *filp, char __user *buf, size_t count, loff_t *f_pos)
>  
>  	mutex_lock(&spidev->buf_lock);
>  	status = spidev_sync_read(spidev, count);
> -	if (status == 0) {
> +	if (status > 0) {
>  		unsigned long	missing;
>  
> -		missing = copy_to_user(buf, spidev->buffer, count);
> -		if (count && missing == count)
> +		missing = copy_to_user(buf, spidev->buffer, status);
> +		if (status && missing == status)
>  			status = -EFAULT;
>  		else
> -			status = count - missing;
> +			status = status - missing;
>  	}
>  	mutex_unlock(&spidev->buf_lock);
>  
> @@ -200,8 +200,6 @@ spidev_write(struct file *filp, const char __user *buf,
>  	missing = copy_from_user(spidev->buffer, buf, count);
>  	if (missing == 0) {
>  		status = spidev_sync_write(spidev, count);
> -		if (status == 0)
> -			status = count;
>  	} else
>  		status = -EFAULT;
>  	mutex_unlock(&spidev->buf_lock);
> -- 
> 1.5.5.2
> 



-------------------------------------------------------------------------
Sponsored by: SourceForge.net Community Choice Awards: VOTE NOW!
Studies have shown that voting for your favorite open source project,
along with a healthy diet, reduces your potential for chronic lameness
and boredom. Vote Now at http://www.sourceforge.net/community/cca08