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 >
less(1): refreshing file of size 0 results in file being treated as a pipe
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