lkml.org 
[lkml]   [2001]   [Jun]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH] proc_file_read() (Was: Re: proc_file_read() question)
On Tue, 26 Jun 2001, Jonathan Lundell wrote:

> I use the hack myself, to implement a record-oriented file where the
> file position is a record number. I could probably live with
> PAGE_SIZE, but the current hack works fine with start bigger than
> that, and it's possible that someone counts on it.

Ok, let's use PAGE_OFFSET instead of PAGE_SIZE, then (see new patch
below).

Unless I'm mislead, legitimate values of "start" as a pointer are always
larger than that, and I can hardly imagin e a case where the "unsigned
int" value of start must be greater than PAGE_OFFSET.

I insist that relying on the comparison of two pointers is the wrong
thing. If (as you suggest) the major use of "start" has migrated from the
original intention to that of the "hack", this should be reflected
in the interface by making the "start" parameter to read_proc ()
an unsigned long. Everything else is misleading and error-prone.
For now, "start" is a char* and should be treated as such.

> But if you're allocating your own buffer, you'd probably be better
> off writing your own file ops, and not using the default
> proc_file_read() at all. At the very least you'd save a redundant
> __get_free_page/free_page pair.

That's right, but nevertheless (repeat) comparing "start" and "page" is
wrong.

Regards,
Martin

--
Martin Wilck <Martin.Wilck@fujitsu-siemens.com>
FSC EP PS DS1, Paderborn Tel. +49 5251 8 15113

--- linux-2.4.5/fs/proc/generic.c Mon Jun 25 13:46:26 2001
+++ 2.4.5mw/fs/proc/generic.c Wed Jun 27 11:22:14 2001
@@ -104,14 +104,14 @@
* return the bytes, and set `start' to the desired offset
* as an unsigned int. - Paul.Russell@rustcorp.com.au
*/
- n -= copy_to_user(buf, start < page ? page : start, n);
+ n -= copy_to_user(buf, (unsigned long) start < PAGE_OFFSET ? page : start, n);
if (n == 0) {
if (retval == 0)
retval = -EFAULT;
break;
}

- *ppos += start < page ? (long)start : n; /* Move down the file */
+ *ppos += (unsigned long) start < PAGE_OFFSET ? (unsigned long) start : n; /* Move down the file */
nbytes -= n;
buf += n;
retval += n;


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-03-22 12:55    [W:1.196 / U:0.012 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site