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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/
