proc_file_read bug?
I don't understand this part of proc_file_read() in 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]
*/
n -= copy_to_user(buf, start < page ? page : start, n);
if (n == 0) {
if (retval == 0)
retval = -EFAULT;
break;
}
*ppos += start < page ? (long)start : n; /* Move down the file */
nbytes -= n;
buf += n;
retval += n;When start >= page, we copy n bytes beginning at start and increase *ppos by n. Makes sense. But what happens when start < page? We will copy n bytes starting at page, then increase *ppos by start!! What sense does that make? If there's cleverness happening here, someone please document it.
For your convenience, I append the whole existing proc_file_read function below.
--
Thomas
static ssize_tproc_file_read(struct file * file, char * buf, size_t nbytes, loff_t *ppos)
{
struct inode * inode = file->f_dentry->d_inode;
char *page;
ssize_t retval=0;
int eof=0;
ssize_t n, count;
char *start;
struct proc_dir_entry * dp;
dp = (struct proc_dir_entry *) inode->u.generic_ip;
if (!(page = (char*) __get_free_page(GFP_KERNEL)))
return -ENOMEM;
while ((nbytes > 0) && !eof)
{
count = MIN(PROC_BLOCK_SIZE, nbytes);
start = NULL;
if (dp->get_info) {
/*
* 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) {
n = dp->read_proc(page, &start, *ppos,
count, &eof, dp->data);
} else
break;
if (!start) {
/*
* For proc files that are less than 4k
*/
start = page + *ppos;
n -= *ppos;
if (n <= 0)
break;
if (n > count)
n = count;
}
if (n == 0)
break; /* End of file */
if (n < 0) {
if (retval == 0)
retval = n;
break;
}
/* 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]
*/
n -= copy_to_user(buf, start < page ? page : start, n);
if (n == 0) {
if (retval == 0)
retval = -EFAULT;
break;
}
*ppos += start < page ? (long)start : n; /* Move down the file */
nbytes -= n;
buf += n;
retval += n;
}
free_page((unsigned long) page);
return retval;
}Thomas Hood <[email protected]> writes:
|> I don't understand this part of proc_file_read() in |> 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] |> */ |> n -= copy_to_user(buf, start < page ? page : start, n); |> if (n == 0) { |> if (retval == 0) |> retval = -EFAULT; |> break; |> } |> |> *ppos += start < page ? (long)start : n; /* Move down the file */ |> nbytes -= n; |> buf += n; |> retval += n; |> |> When start >= page, we copy n bytes beginning at start and |> increase *ppos by n. Makes sense. But what happens when |> start < page? We will copy n bytes starting at page, then |> increase *ppos by start!! What sense does that make? If |> there's cleverness happening here, someone please document it.
It is documented, RTFC.
Andreas.
Signature · 4 lines
Andreas Schwab, SuSE Labs, [email protected] SuSE GmbH, Deutschherrnstr. 15-19, D-90429 Nürnberg Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different."
Quoted 7 lines · Andreas Schwab
|> /* 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] |> */It is documented, RTFC.
Comment or Code? The comment is somewhat ambiguous and incorrect.
Reading the code, I take it that "start" is either a pointer into the buffer where the string of n data bytes starts, or else (when it is assigned a value less than the beginning of the buffer) it is a special value by which the file offset is to be adjusted, instead of n. Thus the comment might be clarified:
/*
* This is a hack to allow adjusting the file offset
* by a number different from the number of bytes read.
* Simply place the data at page, return the number of
* bytes read, and set "start" to the (signed long) amount
* by which the file offset is to be increased or
* decreased
*/
My question then is: why would one want to adjust the file offset other than by +n?
--
Thomas