Re: generic_file_write code segment in 2.2.18

[Stephen C. Tweedie]

Hi,

On Thu, Jan 04, 2001 at 08:51:37AM +0100, Andi Kleen wrote:
> On Wed, Jan 03, 2001 at 09:29:48PM -0800, Asang K Dani wrote:
> 
> The code is buggy as far as I can see. copy_from_user doesn't return the 
> number of bytes copied, but the number of bytes not copied when an error
> occurs (or 0 on success).  

But in this case, _any_ non-zero number of bytes copied is a success,
because it indicates that we have dirtied a portion of the page cache.

> Correct would be:
> 
> 	
> --- linux-work/mm/filemap.c-o	Wed Jan  3 17:37:27 2001
>  		dest = (char *) page_address(page) + offset;
>  		if (dest != buf) { /* See comment in update_vm_cache_cond. */
> -			bytes -= copy_from_user(dest, buf, bytes);
> +			if (copy_from_user(dest, buf, bytes))
> +				status = -EFAULT; 
>  			flush_dcache_page(page_address(page));
>  		}
> -		status = -EFAULT;
> -		if (bytes)
> +		if (!status)
>  			status = inode->i_op->updatepage(file, page, offset, bytes, sync);

No, because then you'd be skipping the updatepage() call if we took a
fault mid-copy after copying some data.  That would imply you had
dirtied the page cache without an updatepage().

The current behaviour should just result in a short IO, which should
be fine.

--Stephen