Index: fs/proc/generic.c
===================================================================
RCS file: /cvs/linuxppc64/linuxppc64_2_4/fs/proc/generic.c,v
retrieving revision 1.6
diff -u -r1.6 generic.c
--- fs/proc/generic.c 8 Oct 2001 19:19:59 -0000 1.6
+++ fs/proc/generic.c 22 Mar 2002 14:34:47 -0000
@@ -104,14 +104,14 @@
* return the bytes, and set `start' to the desired offset
* as an unsigned int. - [email protected]
*/
- n -= copy_to_user(buf, start < page ? page : start, n);
+ n -= copy_to_user(buf, (unsigned long)start < PROC_BLOCK_SIZE ? 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 < PROC_BLOCK_SIZE ? (long)start : n; /* Move down the file */
nbytes -= n;
buf += n;
retval += n;
In http://marc.theaimsgroup.com/?l=linux-kernel&m=101304602932539&w=2
I posted a patch which includes the following comment:
+ /*
+ * How to be a proc read function
+ * ------------------------------
+ * Prototype:
+ * int f(char *buffer, char **start, off_t offset,
+ * int count, int *peof, void *dat)
+ *
+ * Assume that the buffer is "count" bytes in size.
+ *
+ * If you know you have supplied all the data you
+ * have, set *peof.
+ *
+ * You have three ways to return data:
+ * 0) Leave *start = NULL. (This is the default.)
+ * Put the data of the requested offset at that
+ * offset within the buffer. Return the number (n)
+ * of bytes there are from the beginning of the
+ * buffer up to the last byte of data. If the
+ * number of supplied bytes (= n - offset) is
+ * greater than zero and you didn't signal eof
+ * and the reader is prepared to take more data
+ * you will be called again with the requested
+ * offset advanced by the number of bytes
+ * absorbed. This interface is useful for files
+ * no larger than the buffer.
+ * 1) Set *start = an unsigned long value less than
+ * the buffer address but greater than zero.
+ * Put the data of the requested offset at the
+ * beginning of the buffer. Return the number of
+ * bytes of data placed there. If this number is
+ * greater than zero and you didn't signal eof
+ * and the reader is prepared to take more data
+ * you will be called again with the requested
+ * offset advanced by *start. This interface is
+ * useful when you have a large file consisting
+ * of a series of blocks which you want to count
+ * and return as wholes.
+ * (Hack by [email protected])
+ * 2) Set *start = an address within the buffer.
+ * Put the data of the requested offset at *start.
+ * Return the number of bytes of data placed there.
+ * If this number is greater than zero and you
+ * didn't signal eof and the reader is prepared to
+ * take more data you will be called again with the
+ * requested offset advanced by the number of bytes
+ * absorbed.
+ */
This patch was included in David Jones's patches for the 2.5
series.
> I'm trying to understand this little hack in 2.4.18's (and earlier)
> fs/proc/generic.c:
> /* This is a hack to allow mangling of file pos independent
> * of actual bytes read. Simply place the data at page,
> * return the bytes, and set `start' to the desired offset
> * as an unsigned int. - [email protected]
> */
> I take it to mean that if I return a small integer for "start" instead
> of a ptr the hack will kick in. However it compares pointers instead.
That's why it's called a "hack".
> I'll attach a patch that does it the way I am thinking -- but I may be
> misunderstanding the comment.
You are misunderstanding it.
--
Thomas Hood
__________________________________________________
Do You Yahoo!?
Everything you'll ever need on one web page
from News and Sport to Email and Music Charts
http://uk.my.yahoo.com
J.D. Hood wrote:
> I posted a patch which includes the following comment:
>
> + /*
> + * How to be a proc read function
> + * ------------------------------
[...]
How about applying my trivial patch and then adding this to your nice
comment?
3) Set *start = an address outside the buffer.
Put the data of the requested offset at *start.
Return the number of bytes of data placed there.
If this number is greater than zero and you
didn't signal eof and the reader is prepared to
take more data you will be called again with the
requested offset advanced by the number ob tyes
absorbed.
The code should still work with the other cases now that the hack is
fixed. Of course, rather than add 3), it would be better to re-word 2)
(e.g. "Set *start = address of the buffer which may or may not be in the
given buffer.).
There are cases where the data is available and need not be copied. My
code got simpler when I got rid of the need to copy my data around.
-todd
Unfortunately, your method #3 conflicts with methods #0 through #2,
which exhaust the range of possible values that may be returned
in *start. Any value greater than buffer is regarded as being
"within the buffer".
Introducing method #1 was a bad idea because this hack made it
impossible cleanly to implement what you suggest.
--
Thomas Hood
On Mon, 2002-03-25 at 13:18, Todd Inglett wrote:
> How about applying my trivial patch and then adding this to your nice
> comment?
>
> 3) Set *start = an address outside the buffer.
> Put the data of the requested offset at *start.
> Return the number of bytes of data placed there.
> If this number is greater than zero and you
> didn't signal eof and the reader is prepared to
> take more data you will be called again with the
> requested offset advanced by the number ob tyes
> absorbed.
>
> The code should still work with the other cases now that the hack is
> fixed. Of course, rather than add 3), it would be better to re-word 2)
> (e.g. "Set *start = address of the buffer which may or may not be in the
> given buffer.).
>
> There are cases where the data is available and need not be copied. My
> code got simpler when I got rid of the need to copy my data around.
Index: fs/proc/generic.c
===================================================================
RCS file: /cvs/linuxppc64/linuxppc64_2_4/fs/proc/generic.c,v
retrieving revision 1.6
diff -u -r1.6 generic.c
--- fs/proc/generic.c 8 Oct 2001 19:19:59 -0000 1.6
+++ fs/proc/generic.c 22 Mar 2002 14:34:47 -0000
@@ -104,14 +104,14 @@
* return the bytes, and set `start' to the desired offset
* as an unsigned int. - [email protected]
*/
- n -= copy_to_user(buf, start < page ? page : start, n);
+ n -= copy_to_user(buf, (unsigned long)start < PROC_BLOCK_SIZE ? 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 < PROC_BLOCK_SIZE ? (long)start : n; /* Move down the file */
nbytes -= n;
buf += n;
retval += n;
On Wed, 2002-03-27 at 13:26, Todd Inglett wrote:
> I guess I don't understand the conflict.
There are three cases:
0) start == 0
1) 0 < start < buffer
2) start >= buffer
These exhaust all the possible values that can be returned
in *start.
You propose to change the code so that there are three cases:
0) start == 0
1') 0 < start < PROC_BLOCK_SIZE
2'/3) start >= PROC_BLOCK_SIZE
However, we can't make the change you propose because it would
break functions that use case #1 with a *start value greater
than PROC_BLOCK_SIZE.
>... is there a chance that start >= PROC_BLOCK_SIZE (but start < page)
> in case #1?
Yes.
> If that is true I am wondering how it could possibly be correct
> since start will be used as a length which is greater than the
> size of the page.
start will be used as an offset, not as a length.
If you think the hack was a bad idea, I agree with you.
But we can't change it without auditing all the proc read
functions that use case #1.
--
Thomas Hood
Thomas Hood wrote:
>>If that is true I am wondering how it could possibly be correct
>>since start will be used as a length which is greater than the
>>size of the page.
>>
>
> start will be used as an offset, not as a length.
>
> If you think the hack was a bad idea, I agree with you.
> But we can't change it without auditing all the proc read
> functions that use case #1.
Ahh...thanks for taking the trouble to answer all these questions :). I
obviously wasn't paying attention enough to see that start was *not*
used as a length. Sigh. Anyway, yes I agree we cannot patch this
without some serious review and I'm not going to volunteer today :).
-todd