lkml.org 
[lkml]   [2000]   [Oct]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
Subject[PATCH] 0-byte read()/write() behaviour
Single Unix specifies that 0-byte reads, as well as 0-byte writes, should
"return 0 and have no other results". Our current implementation violates
the first requirement and makes it very easy to violate the second one.

read(page_cache_fd, invalid_ptr, 0) returns -EFAULT; IMHO, this is a
clear bug - suppose you have managed to acquire the page covering virtual
addresses up to and including PAGE_OFFSET-1 (on x86).
read(fd, PAGE_OFFSET-n, n) now is successful for PAGE_SIZE >= n > 0, but
fails with -EFAULT for n = 0.

looking through drivers/char/*.c, many drivers seem to behave unexpectedly
for a 0-byte read; this includes code that overwrites the byte read()'s
second argument points to, endless loops, and several different error
returns.

generic_file_read and generice_file_write, which I suppose are the most
critical functions for read()/write() performance, both have special cases
to handle the count == 0 case.


So I think modifying sys_read and sys_write to take care of the count == 0
case and only calling the file-specific function when it's actually
necessary makes most sense; as generic_file_(read|write) get simplified
accordingly, there should be no performance impact.

Linus ?

patch against test10-pre4

diff -ur linux/fs/read_write.c linux-prumpf/fs/read_write.c
--- linux/fs/read_write.c Fri Oct 20 04:52:58 2000
+++ linux-prumpf/fs/read_write.c Fri Oct 20 06:03:23 2000
@@ -120,6 +120,10 @@
ssize_t ret;
struct file * file;

+ /* Required for SU compatibility. It also simplifies generic_file_read */
+ if (!count)
+ return 0;
+
ret = -EBADF;
file = fget(fd);
if (file) {
@@ -145,6 +149,10 @@
{
ssize_t ret;
struct file * file;
+
+ /* Required for SU compatibility. */
+ if (!count)
+ return 0;

ret = -EBADF;
file = fget(fd);
diff -ur linux/mm/filemap.c linux-prumpf/mm/filemap.c
--- linux/mm/filemap.c Fri Oct 20 05:05:04 2000
+++ linux-prumpf/mm/filemap.c Fri Oct 20 06:08:41 2000
@@ -1124,27 +1124,18 @@
*/
ssize_t generic_file_read(struct file * filp, char * buf, size_t count, loff_t *ppos)
{
- ssize_t retval;
+ read_descriptor_t desc;

- retval = -EFAULT;
- if (access_ok(VERIFY_WRITE, buf, count)) {
- retval = 0;
-
- if (count) {
- read_descriptor_t desc;
-
- desc.written = 0;
- desc.count = count;
- desc.buf = buf;
- desc.error = 0;
- do_generic_file_read(filp, ppos, &desc, file_read_actor);
-
- retval = desc.written;
- if (!retval)
- retval = desc.error;
- }
- }
- return retval;
+ if (!access_ok(VERIFY_WRITE, buf, count))
+ return -EFAULT;
+
+ desc.written = 0;
+ desc.count = count;
+ desc.buf = buf;
+ desc.error = 0;
+ do_generic_file_read(filp, ppos, &desc, file_read_actor);
+
+ return desc.written ? : desc.error;
}

static int file_send_actor(read_descriptor_t * desc, struct page *page, unsigned long offset , unsigned long size)
@@ -2424,13 +2415,12 @@
}

status = 0;
- if (count) {
- remove_suid(inode);
- inode->i_ctime = inode->i_mtime = CURRENT_TIME;
- mark_inode_dirty(inode);
- }

- while (count) {
+ remove_suid(inode);
+ inode->i_ctime = inode->i_mtime = CURRENT_TIME;
+ mark_inode_dirty(inode);
+
+ do {
unsigned long bytes, index, offset;
char *kaddr;

@@ -2480,7 +2470,7 @@

if (status < 0)
break;
- }
+ } while (count);
*ppos = pos;

if (cached_page)
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/
\
 
 \ /
  Last update: 2005-03-22 12:41    [W:0.054 / U:0.356 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site