* [PATCH] seq_file:update file->f_pos when lseek() to m->read_pos
@ 2013-06-29 9:11 Jiaxing Wang
2013-07-01 12:41 ` fangdong
2013-07-01 12:52 ` Dong Fang
0 siblings, 2 replies; 5+ messages in thread
From: Jiaxing Wang @ 2013-06-29 9:11 UTC (permalink / raw)
To: viro; +Cc: Jiaxing Wang, linux-kernel, linux-fsdevel
After pread(), file->f_pos and m->read_pos get different,
and lseek() to m->read_pos did not update file->f_pos, then
a subsequent read may read from a wrong position, the following
program shows the problem:
char str1[32] = { 0 };
char str2[32] = { 0 };
int poffset = 10;
int count = 20;
/*open any seq file*/
int fd = open("/proc/modules", O_RDONLY);
pread(fd, str1, count, poffset);
printf("pread:%s\n", str1);
/*seek to where m->read_pos is*/
lseek(fd, poffset+count, SEEK_SET);
/*supposed to read from poffset+count, but this read from position 0*/
read(fd, str2, count);
printf("read:%s\n", str2);
Signed-off-by: Jiaxing Wang <hello.wjx@gmail.com>
---
fs/seq_file.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/fs/seq_file.c b/fs/seq_file.c
index 774c1eb..4b22b26 100644
--- a/fs/seq_file.c
+++ b/fs/seq_file.c
@@ -328,7 +328,8 @@ loff_t seq_lseek(struct file *file, loff_t offset, int whence)
m->read_pos = offset;
retval = file->f_pos = offset;
}
- }
+ } else
+ file->f_pos = offset;
}
file->f_version = m->version;
mutex_unlock(&m->lock);
--
1.8.1.2
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] seq_file:update file->f_pos when lseek() to m->read_pos 2013-06-29 9:11 [PATCH] seq_file:update file->f_pos when lseek() to m->read_pos Jiaxing Wang @ 2013-07-01 12:41 ` fangdong 2013-07-02 0:43 ` Jiaxing Wang 2013-07-01 12:52 ` Dong Fang 1 sibling, 1 reply; 5+ messages in thread From: fangdong @ 2013-07-01 12:41 UTC (permalink / raw) To: Jiaxing Wang; +Cc: viro, linux-kernel, linux-fsdevel On 06/29/2013 05:11 AM, Jiaxing Wang wrote: > After pread(), file->f_pos and m->read_pos get different, > and lseek() to m->read_pos did not update file->f_pos, then > a subsequent read may read from a wrong position, the following > program shows the problem: > > char str1[32] = { 0 }; > char str2[32] = { 0 }; > int poffset = 10; > int count = 20; > > /*open any seq file*/ > int fd = open("/proc/modules", O_RDONLY); > > pread(fd, str1, count, poffset); > printf("pread:%s\n", str1); > > /*seek to where m->read_pos is*/ > lseek(fd, poffset+count, SEEK_SET); > > /*supposed to read from poffset+count, but this read from position 0*/ > read(fd, str2, count); > printf("read:%s\n", str2); > > Signed-off-by: Jiaxing Wang <hello.wjx@gmail.com> > --- > fs/seq_file.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/fs/seq_file.c b/fs/seq_file.c > index 774c1eb..4b22b26 100644 > --- a/fs/seq_file.c > +++ b/fs/seq_file.c > @@ -328,7 +328,8 @@ loff_t seq_lseek(struct file *file, loff_t offset, int whence) > m->read_pos = offset; > retval = file->f_pos = offset; > } > - } > + } else > + file->f_pos = offset; > } > file->f_version = m->version; > mutex_unlock(&m->lock); > This does not appear to be a problem, in linux man page, the behaver seems clearly defined: DESCRIPTION pread() reads up to count bytes from file descriptor fd at offset off-set (from the start of the file) into the buffer starting at buf. The file offset is not changed. pwrite() writes up to count bytes from the buffer starting at buf to the file descriptor fd at offset offset. The file offset is not changed. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] seq_file:update file->f_pos when lseek() to m->read_pos 2013-07-01 12:41 ` fangdong @ 2013-07-02 0:43 ` Jiaxing Wang 2013-07-04 2:00 ` Gu Zheng 0 siblings, 1 reply; 5+ messages in thread From: Jiaxing Wang @ 2013-07-02 0:43 UTC (permalink / raw) To: fangdong; +Cc: Jiaxing Wang, viro, linux-kernel, linux-fsdevel On 07/01/2013 08:41 PM, fangdong wrote: > On 06/29/2013 05:11 AM, Jiaxing Wang wrote: >> After pread(), file->f_pos and m->read_pos get different, >> and lseek() to m->read_pos did not update file->f_pos, then >> a subsequent read may read from a wrong position, the following >> program shows the problem: >> >> char str1[32] = { 0 }; >> char str2[32] = { 0 }; >> int poffset = 10; >> int count = 20; >> >> /*open any seq file*/ >> int fd = open("/proc/modules", O_RDONLY); >> >> pread(fd, str1, count, poffset); >> printf("pread:%s\n", str1); >> >> /*seek to where m->read_pos is*/ >> lseek(fd, poffset+count, SEEK_SET); >> >> /*supposed to read from poffset+count, but this read from position 0*/ >> read(fd, str2, count); >> printf("read:%s\n", str2); >> >> Signed-off-by: Jiaxing Wang <hello.wjx@gmail.com> >> --- >> fs/seq_file.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/fs/seq_file.c b/fs/seq_file.c >> index 774c1eb..4b22b26 100644 >> --- a/fs/seq_file.c >> +++ b/fs/seq_file.c >> @@ -328,7 +328,8 @@ loff_t seq_lseek(struct file *file, loff_t offset, int whence) >> m->read_pos = offset; >> retval = file->f_pos = offset; >> } >> - } >> + } else >> + file->f_pos = offset; >> } >> file->f_version = m->version; >> mutex_unlock(&m->lock); >> > This does not appear to be a problem, in linux man page, the behaver seems clearly defined: > > DESCRIPTION > pread() reads up to count bytes from file descriptor fd at offset off-set (from the start of the file) into the buffer starting at buf. The file offset is not changed. > > pwrite() writes up to count bytes from the buffer starting at buf to the file descriptor fd at offset offset. The file offset is not changed. > > -- > 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/ There's no problem that pread() don't change file->f_pos, but I think lseek() should have changed it. Any comments from other people, and Alexander? ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] seq_file:update file->f_pos when lseek() to m->read_pos 2013-07-02 0:43 ` Jiaxing Wang @ 2013-07-04 2:00 ` Gu Zheng 0 siblings, 0 replies; 5+ messages in thread From: Gu Zheng @ 2013-07-04 2:00 UTC (permalink / raw) To: Jiaxing Wang; +Cc: fangdong, Jiaxing Wang, viro, linux-kernel, linux-fsdevel Hi Jiaxing, Please refer to inline comment.:) On 07/02/2013 08:43 AM, Jiaxing Wang wrote: > On 07/01/2013 08:41 PM, fangdong wrote: >> On 06/29/2013 05:11 AM, Jiaxing Wang wrote: >>> After pread(), file->f_pos and m->read_pos get different, >>> and lseek() to m->read_pos did not update file->f_pos, then >>> a subsequent read may read from a wrong position, the following >>> program shows the problem: >>> >>> char str1[32] = { 0 }; >>> char str2[32] = { 0 }; >>> int poffset = 10; >>> int count = 20; >>> >>> /*open any seq file*/ >>> int fd = open("/proc/modules", O_RDONLY); >>> >>> pread(fd, str1, count, poffset); >>> printf("pread:%s\n", str1); >>> >>> /*seek to where m->read_pos is*/ >>> lseek(fd, poffset+count, SEEK_SET); >>> >>> /*supposed to read from poffset+count, but this read from position 0*/ >>> read(fd, str2, count); >>> printf("read:%s\n", str2); >>> >>> Signed-off-by: Jiaxing Wang <hello.wjx@gmail.com> >>> --- >>> fs/seq_file.c | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/fs/seq_file.c b/fs/seq_file.c >>> index 774c1eb..4b22b26 100644 >>> --- a/fs/seq_file.c >>> +++ b/fs/seq_file.c >>> @@ -328,7 +328,8 @@ loff_t seq_lseek(struct file *file, loff_t offset, int whence) >>> m->read_pos = offset; >>> retval = file->f_pos = offset; >>> } >>> - } >>> + } else >>> + file->f_pos = offset; >>> } >>> file->f_version = m->version; >>> mutex_unlock(&m->lock); >>> >> This does not appear to be a problem, in linux man page, the behaver seems clearly defined: >> >> DESCRIPTION >> pread() reads up to count bytes from file descriptor fd at offset off-set (from the start of the file) into the buffer starting at buf. The file offset is not changed. >> >> pwrite() writes up to count bytes from the buffer starting at buf to the file descriptor fd at offset offset. The file offset is not changed. >> >> -- >> 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/ > There's no problem that pread() don't change file->f_pos, but I think lseek() should have changed it. No, the way that seq_seek deal with pos is right. Remember this seq file, it is an iterator interface, e.g. it provides infos of each element of a list, so the we should make sure that the info we read from seq file is the whole infos of each entry, so seq_lseek can not set pos to a optional point, only can on the points that divided by a whole infos size of each entry. If we can not set pos on the special position, ste it as 0, I think it's reasonable. Thanks, Gu > Any comments from other people, and Alexander? > -- > 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/ > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] seq_file:update file->f_pos when lseek() to m->read_pos 2013-06-29 9:11 [PATCH] seq_file:update file->f_pos when lseek() to m->read_pos Jiaxing Wang 2013-07-01 12:41 ` fangdong @ 2013-07-01 12:52 ` Dong Fang 1 sibling, 0 replies; 5+ messages in thread From: Dong Fang @ 2013-07-01 12:52 UTC (permalink / raw) To: Jiaxing Wang; +Cc: viro, linux-kernel, linux-fsdevel On 06/29/2013 05:11 AM, Jiaxing Wang wrote: > After pread(), file->f_pos and m->read_pos get different, > and lseek() to m->read_pos did not update file->f_pos, then > a subsequent read may read from a wrong position, the following > program shows the problem: > > char str1[32] = { 0 }; > char str2[32] = { 0 }; > int poffset = 10; > int count = 20; > > /*open any seq file*/ > int fd = open("/proc/modules", O_RDONLY); > > pread(fd, str1, count, poffset); > printf("pread:%s\n", str1); > > /*seek to where m->read_pos is*/ > lseek(fd, poffset+count, SEEK_SET); > > /*supposed to read from poffset+count, but this read from position 0*/ > read(fd, str2, count); > printf("read:%s\n", str2); > > Signed-off-by: Jiaxing Wang <hello.wjx@gmail.com> > --- > fs/seq_file.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/fs/seq_file.c b/fs/seq_file.c > index 774c1eb..4b22b26 100644 > --- a/fs/seq_file.c > +++ b/fs/seq_file.c > @@ -328,7 +328,8 @@ loff_t seq_lseek(struct file *file, loff_t offset, int whence) > m->read_pos = offset; > retval = file->f_pos = offset; > } > - } > + } else > + file->f_pos = offset; > } > file->f_version = m->version; > mutex_unlock(&m->lock); > This does not appear to be a problem, in linux man page, the behaver seems clearly defined: DESCRIPTION pread() reads up to count bytes from file descriptor fd at offset off-set (from the start of the file) into the buffer starting at buf. The file offset is not changed. pwrite() writes up to count bytes from the buffer starting at buf to the file descriptor fd at offset offset. The file offset is not changed. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-07-04 2:03 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-06-29 9:11 [PATCH] seq_file:update file->f_pos when lseek() to m->read_pos Jiaxing Wang 2013-07-01 12:41 ` fangdong 2013-07-02 0:43 ` Jiaxing Wang 2013-07-04 2:00 ` Gu Zheng 2013-07-01 12:52 ` Dong Fang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox