"A month of sundays ago Mike Galbraith wrote:"
> That section of code is beautiful if you're reading an array :)
No it's not. Out of annoyance at its obscurantism I've spent a few
hours un-obscuring it. As far as I can see, there are three main modes
of use of that code, all but one undocumented. There might be six
modes. Can you look over the patch below and make sure that it
preserves the functionality exactly? I didn't make semantic changes.
Only comments and restructuring, and abstraction (reverse engineering)
to make the modes explicit by putting in variables "style" and "mode"
that control them.
The patch below is the same for 2.2.* as for 2.3.44, apart from spaces.
Please look at it, Can you comment on the FIXMEs? These are not
things I've introduced, just things I noticed!
I've checked that at least 2.2.10 and 13 compile and run with it.
Peter
--- linux-2.3.44/fs/proc/generic.c.orig Wed Feb 9 20:42:35 2000
+++ linux-2.3.44/fs/proc/generic.c Sun Feb 13 19:32:12 2000
@@ -60,64 +60,203 @@
struct proc_dir_entry * dp;
dp = (struct proc_dir_entry *) inode->u.generic_ip;
- if (!(page = (char*) __get_free_page(GFP_KERNEL)))
+ page = (char*) __get_free_page(GFP_KERNEL);
+ if (!page)
return -ENOMEM;
- while ((nbytes > 0) && !eof)
+ /*
+ * comments and slight cleanup by ptb@it.uc3m.es. 13/2/00
+ */
+ while (nbytes > 0 && !eof)
{
+ /*
+ * style is 1 for read using dp->get_info (back compat)
+ * 2 for read using dp->read_proc
+ * 0 if neither and so we want to get out of here
+ */
+ int style = dp->get_info ? 1 : (dp->read_proc ? 2 : 0);
+ /*
+ * mode is 0 for reading from a short (4k) file (start = 0)
+ * 1 for reading from a long file (start < pagesize)
+ * 2 for something very general (start points in page)
+ * we modify this according to value of start.
+ */
+ int mode = 0;
+
+ /*
+ * ask for count many chars this time round
+ */
count = MIN(PROC_BLOCK_SIZE, nbytes);
+ /*
+ * start is a modeswitcher. It gets written to by the
+ * called routines to tell us to change our behaviour.
+ */
start = NULL;
- if (dp->get_info) {
+
+ if (!style)
+ break;
+
+ switch(style) {
+ case 1:
/*
+ * dp->get_info is nonzero
+ *
* Handle backwards compatibility with the old net
* routines.
*/
n = dp->get_info(page, &start, *ppos, count);
if (n < count)
eof = 1;
- } else if (dp->read_proc) {
+ break;
+
+ case 2:
+ /*
+ * dp->read_proc is nonzero
+ */
n = dp->read_proc(page, &start, *ppos,
count, &eof, dp->data);
- } else
- break;
+ break;
+ }
- if (!start) {
+ /*
+ * From here on there are two modes of operation posssible.
+ *
+ * If start is returned as 0, it means that the routine
+ * wants to scribble on a single page. We repeatedly
+ * request all its data en bloc, throwing away the stuff
+ * we already had, until we have got all of it. Mode 0.
+ *
+ * If start is returned nonzero (and < pagesize), it
+ * means the routine is prepared to play with us on
+ * more than one page. Mode 1.
+ */
+
+ mode = (start == 0) ? 0 : (start < page ? 1 : 2) ;
+
+ switch (mode) {
+ case 0:
+ if (!start) {
/*
* For proc files that are less than 4k
+ *
+ * We start looking in the returned buffer at the
+ * current file offset.
*/
start = page + *ppos;
- n -= *ppos;
+ /*
+ * We only look at the new chars received
+ * beyond those we had already. If there
+ * are none, we will consider ourselves finished.
+ */
+ n -= *ppos;
if (n <= 0)
- break;
+ goto exit;
+ /*
+ * Can't have received more bytes than we asked
+ * for, so say we received just so many. Surely
+ * this is impossible! Signs? FIXME.
+ */
+ }
+
if (n > count)
n = count;
+ break;
+
+ /* default outside loop catches fallthru from above */
}
+
+ /*
+ * "Ordinary" (i.e. long) file processing starts here.
+ */
+
if (n == 0)
- break; /* End of file */
+ break; /* No (new) bytes received. End of file */
+
+ /*
+ * Stop on first error.
+ */
if (n < 0) {
if (retval == 0)
retval = n;
break;
}
-
- /* This is a hack to allow mangling of file pos independent
+
+ /*
+ * 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. - Paul.Russell@rustcorp.com.au
- */
- n -= copy_to_user(buf, start < page ? page : start, n);
+ *
+ * Normally, start is zero, so we read on from page. Mode 0.
+ * In the long file case, start is a small offset, so
+ * we still read on from page. Mode 1.
+ * In a third scenario, the routine really returned
+ * a pointer into the page to start reading from (ha ha)
+ * so we read on from there. Mode 2.
+ */
+ switch (mode) {
+ case 0:
+ case 1:
+ n -= copy_to_user(buf, page, n);
+ break;
+
+ case 2:
+ default:
+ n -= copy_to_user(buf, start, n);
+ break;
+ }
+
+ /*
+ * We stop with EFAULT if we copied all bytes
+ * to user space on first attempt! Why? FIXME
+ */
if (n == 0) {
if (retval == 0)
retval = -EFAULT;
break;
}
+
+ /*
+ * Move down the file.
+ */
+ switch (mode) {
+
+ case 1:
+ *ppos += (long)start;
+ break;
+
+ case 0:
+ case 2:
+ /*
+ * I believe this to be broken. n is the
+ * no. of bytes NOT written to user space
+ * from the buffer. FIXME
+ */
+ *ppos += n;
+ break;
+
+ default:
+ /* supposedly impossible */
+ break;
+ }
+
+ /*
+ * We delivered some (new) bytes successfully. Accounting.
+ *
+ * Urgle. This looks horribly broken. In all reasonable
+ * cases, n will be zero, as it's the diff in bytes
+ * copied to user space from the buffer, and those
+ * that were in the buffer, which we requested
+ * transfered! FIXME
+ */
- *ppos += start < page ? (long)start : n; /* Move down the file */
nbytes -= n;
buf += n;
retval += n;
}
+
+ exit:
free_page((unsigned long) page);
return retval;
}
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Tue Feb 15 2000 - 21:00:25 EST