Re: less(1): refreshing file of size 0 results in file being treated as a pipe
Hi,
thank you for reporting this bug and for providing a patch to fix it.
I just committed your patch.
Also thanks to tb@ and deraadt@ for cross-checking the patch.
Yours,
Ingo
user wrote on Thu, Aug 05, 2021 at 12:43:21AM -0500:
> Oops, forgot that OpenBSD doesn't have ! capability in less. Instead of !echo
> a > % and !echo b > %, run
> $ echo a > /tmp/test
> Press h and q in less to reload the file
> $ echo b > /tmp/test
> Press h and q in less to reload the file
>
> On Thu, Aug 05, 2021 at 12:37:00AM -0500, user wrote:
> > Bug Reproduction:
> > $ touch /tmp/test
> > $ less /tmp/test
> > Press r
> > Run !echo a > %
> > Run !echo b > %
> > Output:
> > a
> > b
> >
> > On Fri, Jul 23, 2021 at 11:15:59AM -0500, user wrote:
> > > Less contains a hack to force files of size 0 to become non-seekable in
> > > order to workaround a linux kernel bug.
> > >
> > > When the file becomes non-seekable any further reads from the file are
> > > appended rather than overwriting the original contents of the file.
> > >
> > > diff --git ch.c ch.c
> > > index 1a679767a42..d7c0aa34e90 100644
> > > --- ch.c
> > > +++ ch.c
> > > @@ -643,19 +643,6 @@ ch_flush(void)
> > > ch_block = 0; /* ch_fpos / LBUFSIZE; */
> > > ch_offset = 0; /* ch_fpos % LBUFSIZE; */
> > >
> > > -#if 1
> > > - /*
> > > -* This is a kludge to workaround a Linux kernel bug: files in
> > > -* /proc have a size of 0 according to fstat() but have readable
> > > -* data. They are sometimes, but not always, seekable.
> > > -* Force them to be non-seekable here.
> > > -*/
> > > - if (ch_fsize == 0) {
> > > - ch_fsize = -1;
> > > - ch_flags &= ~CH_CANSEEK;
> > > - }
> > > -#endif
> > > -
> > > if (lseek(ch_file, (off_t)0, SEEK_SET) == (off_t)-1) {
> > > /*
> > > * Warning only; even if the seek fails for some reason
Re: less(1): refreshing file of size 0 results in file being treated as a pipe
OK deraadt
Ingo Schwarze wrote:
> Hi,
>
> after quite some head-scratching, i consider the following bug report
> legitimate:
>
> user wrote on Thu, Aug 05, 2021 at 12:43:21AM -0500:
> > On Thu, Aug 05, 2021 at 12:37:00AM -0500, user wrote:
> >> On Fri, Jul 23, 2021 at 11:15:59AM -0500, user wrote:
>
> >>> Less contains a hack to force files of size 0 to become non-seekable
> >>> in order to workaround a linux kernel bug.
>
> I'm inserting a few words into the next sentence to make it clearer
> what it is trying to say:
>
> >>> When the file becomes non-seekable any further reads from the file
> >>> are appended
>
> to the temporary buffer in which less(1) holds the content
> of the file
>
> >>> rather than overwriting the original contents of the file
>
> in that buffer.
>
> >> Bug Reproduction:
>
>$ rm -rf /tmp/test
>
> >> $ touch /tmp/test
> >> $ less /tmp/test
> >> Press r
>
> > $ echo a > /tmp/test# my comment: from a different shell
> > Press h and q in less to reload the file
> > $ echo b > /tmp/test
> > Press h and q in less to reload the file
>
> Now less(1) shows the following on the screen because it thinks
> that would be the current content of the file:
>
> >> a
> >> b
>
> That is wrong. Instead, it should show the actual file content,
> which is just:
>
> b
>
> I think the proposed patch makes sense and should be committed:
> File size has nothing to do with whether a file is seekable,
> so i don't think it can cause regressions.
> Also, it fixes the bug described above.
>
> Any developer willing to provide an OK?
> Alternatively, commit yourself with OK schwarze@.
>
> I'm attaching the patch again because the OP mangled it (tabs
> replaced by spaces) and it did not apply.
>
> Yours,
> Ingo
>
>
> Index: ch.c
> ===
> RCS file: /cvs/src/usr.bin/less/ch.c,v
> retrieving revision 1.21
> diff -u -p -r1.21 ch.c
> --- ch.c 3 Sep 2019 23:08:42 - 1.21
> +++ ch.c 6 Aug 2021 16:14:29 -
> @@ -643,19 +643,6 @@ ch_flush(void)
> ch_block = 0; /* ch_fpos / LBUFSIZE; */
> ch_offset = 0; /* ch_fpos % LBUFSIZE; */
>
> -#if 1
> - /*
> - * This is a kludge to workaround a Linux kernel bug: files in
> - * /proc have a size of 0 according to fstat() but have readable
> - * data. They are sometimes, but not always, seekable.
> - * Force them to be non-seekable here.
> - */
> - if (ch_fsize == 0) {
> - ch_fsize = -1;
> - ch_flags &= ~CH_CANSEEK;
> - }
> -#endif
> -
> if (lseek(ch_file, (off_t)0, SEEK_SET) == (off_t)-1) {
> /*
>* Warning only; even if the seek fails for some reason,
>
Re: less(1): refreshing file of size 0 results in file being treated as a pipe
> Any developer willing to provide an OK? ok tb
Re: less(1): refreshing file of size 0 results in file being treated as a pipe
Hi,
after quite some head-scratching, i consider the following bug report
legitimate:
user wrote on Thu, Aug 05, 2021 at 12:43:21AM -0500:
> On Thu, Aug 05, 2021 at 12:37:00AM -0500, user wrote:
>> On Fri, Jul 23, 2021 at 11:15:59AM -0500, user wrote:
>>> Less contains a hack to force files of size 0 to become non-seekable
>>> in order to workaround a linux kernel bug.
I'm inserting a few words into the next sentence to make it clearer
what it is trying to say:
>>> When the file becomes non-seekable any further reads from the file
>>> are appended
to the temporary buffer in which less(1) holds the content
of the file
>>> rather than overwriting the original contents of the file
in that buffer.
>> Bug Reproduction:
$ rm -rf /tmp/test
>> $ touch /tmp/test
>> $ less /tmp/test
>> Press r
> $ echo a > /tmp/test# my comment: from a different shell
> Press h and q in less to reload the file
> $ echo b > /tmp/test
> Press h and q in less to reload the file
Now less(1) shows the following on the screen because it thinks
that would be the current content of the file:
>> a
>> b
That is wrong. Instead, it should show the actual file content,
which is just:
b
I think the proposed patch makes sense and should be committed:
File size has nothing to do with whether a file is seekable,
so i don't think it can cause regressions.
Also, it fixes the bug described above.
Any developer willing to provide an OK?
Alternatively, commit yourself with OK schwarze@.
I'm attaching the patch again because the OP mangled it (tabs
replaced by spaces) and it did not apply.
Yours,
Ingo
Index: ch.c
===
RCS file: /cvs/src/usr.bin/less/ch.c,v
retrieving revision 1.21
diff -u -p -r1.21 ch.c
--- ch.c3 Sep 2019 23:08:42 - 1.21
+++ ch.c6 Aug 2021 16:14:29 -
@@ -643,19 +643,6 @@ ch_flush(void)
ch_block = 0; /* ch_fpos / LBUFSIZE; */
ch_offset = 0; /* ch_fpos % LBUFSIZE; */
-#if 1
- /*
-* This is a kludge to workaround a Linux kernel bug: files in
-* /proc have a size of 0 according to fstat() but have readable
-* data. They are sometimes, but not always, seekable.
-* Force them to be non-seekable here.
-*/
- if (ch_fsize == 0) {
- ch_fsize = -1;
- ch_flags &= ~CH_CANSEEK;
- }
-#endif
-
if (lseek(ch_file, (off_t)0, SEEK_SET) == (off_t)-1) {
/*
* Warning only; even if the seek fails for some reason,
Re: less(1): refreshing file of size 0 results in file being treated as a pipe
Oops, forgot that OpenBSD doesn't have ! capability in less. Instead of !echo a
> % and !echo b > %, run
$ echo a > /tmp/test
Press h and q in less to reload the file
$ echo b > /tmp/test
Press h and q in less to reload the file
On Thu, Aug 05, 2021 at 12:37:00AM -0500, user wrote:
> Bug Reproduction:
> $ touch /tmp/test
> $ less /tmp/test
> Press r
> Run !echo a > %
> Run !echo b > %
> Output:
> a
> b
>
> On Fri, Jul 23, 2021 at 11:15:59AM -0500, user wrote:
> > Less contains a hack to force files of size 0 to become non-seekable in
> > order to workaround a linux kernel bug.
> >
> > When the file becomes non-seekable any further reads from the file are
> > appended rather than overwriting the original contents of the file.
> >
> > diff --git ch.c ch.c
> > index 1a679767a42..d7c0aa34e90 100644
> > --- ch.c
> > +++ ch.c
> > @@ -643,19 +643,6 @@ ch_flush(void)
> > ch_block = 0; /* ch_fpos / LBUFSIZE; */
> > ch_offset = 0; /* ch_fpos % LBUFSIZE; */
> >
> > -#if 1
> > - /*
> > -* This is a kludge to workaround a Linux kernel bug: files in
> > -* /proc have a size of 0 according to fstat() but have readable
> > -* data. They are sometimes, but not always, seekable.
> > -* Force them to be non-seekable here.
> > -*/
> > - if (ch_fsize == 0) {
> > - ch_fsize = -1;
> > - ch_flags &= ~CH_CANSEEK;
> > - }
> > -#endif
> > -
> > if (lseek(ch_file, (off_t)0, SEEK_SET) == (off_t)-1) {
> > /*
> > * Warning only; even if the seek fails for some reason
> >
>
Re: less(1): refreshing file of size 0 results in file being treated as a pipe
Bug Reproduction:
$ touch /tmp/test
$ less /tmp/test
Press r
Run !echo a > %
Run !echo b > %
Output:
a
b
On Fri, Jul 23, 2021 at 11:15:59AM -0500, user wrote:
> Less contains a hack to force files of size 0 to become non-seekable in order
> to workaround a linux kernel bug.
>
> When the file becomes non-seekable any further reads from the file are
> appended rather than overwriting the original contents of the file.
>
> diff --git ch.c ch.c
> index 1a679767a42..d7c0aa34e90 100644
> --- ch.c
> +++ ch.c
> @@ -643,19 +643,6 @@ ch_flush(void)
> ch_block = 0; /* ch_fpos / LBUFSIZE; */
> ch_offset = 0; /* ch_fpos % LBUFSIZE; */
>
> -#if 1
> - /*
> -* This is a kludge to workaround a Linux kernel bug: files in
> -* /proc have a size of 0 according to fstat() but have readable
> -* data. They are sometimes, but not always, seekable.
> -* Force them to be non-seekable here.
> -*/
> - if (ch_fsize == 0) {
> - ch_fsize = -1;
> - ch_flags &= ~CH_CANSEEK;
> - }
> -#endif
> -
> if (lseek(ch_file, (off_t)0, SEEK_SET) == (off_t)-1) {
> /*
> * Warning only; even if the seek fails for some reason
>
