Re: [patch] Remove superfluous seek_filesize() from less

[Ingo Schwarze]

Hi,

On Sat, Sep 16, 2017 at 10:53:47PM +0200, Jesper Wallin wrote:

> I was reading through the code for less/filename.c and noticed that the
> calling for seek_filesize() in filesize() is superfluous?  A wild guess
> is that it's remains from when BAD_LSEEK was removed?

There is more that is useless.

 * The fstat(2) function is required by POSIX, and we couldn't
   care less about non-POSIX systems, so delete the pointless
   comment.

 * The fstat(2) syscall only fails for EBADF and EIO, and POSIX
   would allow failing for EOVERFLOW, too.  In all these cases,
   trying lseek(2) is useless, so simply return error.

OK?
  Ingo


Index: filename.c
===
RCS file: /cvs/src/usr.bin/less/filename.c,v
retrieving revision 1.25
diff -u -p -r1.25 filename.c
--- filename.c  17 Sep 2016 15:06:41 -  1.25
+++ filename.c  28 Oct 2017 14:33:07 -
@@ -363,20 +363,6 @@ bin_file(int f)
 }
 
 /*
- * Try to determine the size of a file by seeking to the end.
- */
-static off_t
-seek_filesize(int f)
-{
-   off_t spos;
-
-   spos = lseek(f, (off_t)0, SEEK_END);
-   if (spos == (off_t)-1)
-   return (-1);
-   return (spos);
-}
-
-/*
  * Read a string from a file.
  * Return a pointer to the string in memory.
  */
@@ -742,7 +728,6 @@ bad_file(char *filename)
 
 /*
  * Return the size of a file, as cheaply as possible.
- * In Unix, we can stat the file.
  */
 off_t
 filesize(int f)
@@ -751,7 +736,7 @@ filesize(int f)
 
if (fstat(f, ) >= 0)
return (statbuf.st_size);
-   return (seek_filesize(f));
+   return (-1);
 }
 
 /*

Re: [patch] Remove superfluous seek_filesize() from less

[Jesper Wallin]

...and a third bump, this time we're actually out of beta for real. ;-)

On Mon, Sep 25, 2017 at 10:19:16AM +0200, Jesper Wallin wrote:
> Shameless bump, now when we're out of beta. :-)
> 
> On Sat, Sep 16, 2017 at 10:53:47PM +0200, Jesper Wallin wrote:
> > Hi all,
> > 
> > I was reading through the code for less/filename.c and noticed that the
> > calling for seek_filesize() in filesize() is superfluous?  A wild guess
> > is that it's remains from when BAD_LSEEK was removed?
> > 
> > 
> > Jesper Wallin
> > 
> > 
> > Index: usr.bin/less/filename.c
> > ===
> > RCS file: /cvs/src/usr.bin/less/filename.c,v
> > retrieving revision 1.25
> > diff -u -p -r1.25 filename.c
> > --- usr.bin/less/filename.c 17 Sep 2016 15:06:41 -  1.25
> > +++ usr.bin/less/filename.c 16 Sep 2017 20:28:34 -
> > @@ -363,20 +363,6 @@ bin_file(int f)
> >  }
> >  
> >  /*
> > - * Try to determine the size of a file by seeking to the end.
> > - */
> > -static off_t
> > -seek_filesize(int f)
> > -{
> > -   off_t spos;
> > -
> > -   spos = lseek(f, (off_t)0, SEEK_END);
> > -   if (spos == (off_t)-1)
> > -   return (-1);
> > -   return (spos);
> > -}
> > -
> > -/*
> >   * Read a string from a file.
> >   * Return a pointer to the string in memory.
> >   */
> > @@ -751,7 +737,7 @@ filesize(int f)
> >  
> > if (fstat(f, ) >= 0)
> > return (statbuf.st_size);
> > -   return (seek_filesize(f));
> > +   return (lseek(f, 0, SEEK_END));
> >  }
> >  
> >  /*

Re: [patch] Remove superfluous seek_filesize() from less

[Jesper Wallin]

On Mon, Sep 25, 2017 at 10:36:18AM +0200, Landry Breuil wrote:
> On Mon, Sep 25, 2017 at 10:19:16AM +0200, Jesper Wallin wrote:
> > Shameless bump, now when we're out of beta. :-)
> 
> being out of beta means we're in release mode..

Oh! Of course, silly me.  Apologize for the noise. 

Re: [patch] Remove superfluous seek_filesize() from less

[Landry Breuil]

On Mon, Sep 25, 2017 at 10:19:16AM +0200, Jesper Wallin wrote:
> Shameless bump, now when we're out of beta. :-)

being out of beta means we're in release mode..

Re: [patch] Remove superfluous seek_filesize() from less

[Jesper Wallin]

Shameless bump, now when we're out of beta. :-)

On Sat, Sep 16, 2017 at 10:53:47PM +0200, Jesper Wallin wrote:
> Hi all,
> 
> I was reading through the code for less/filename.c and noticed that the
> calling for seek_filesize() in filesize() is superfluous?  A wild guess
> is that it's remains from when BAD_LSEEK was removed?
> 
> 
> Jesper Wallin
> 
> 
> Index: usr.bin/less/filename.c
> ===
> RCS file: /cvs/src/usr.bin/less/filename.c,v
> retrieving revision 1.25
> diff -u -p -r1.25 filename.c
> --- usr.bin/less/filename.c   17 Sep 2016 15:06:41 -  1.25
> +++ usr.bin/less/filename.c   16 Sep 2017 20:28:34 -
> @@ -363,20 +363,6 @@ bin_file(int f)
>  }
>  
>  /*
> - * Try to determine the size of a file by seeking to the end.
> - */
> -static off_t
> -seek_filesize(int f)
> -{
> - off_t spos;
> -
> - spos = lseek(f, (off_t)0, SEEK_END);
> - if (spos == (off_t)-1)
> - return (-1);
> - return (spos);
> -}
> -
> -/*
>   * Read a string from a file.
>   * Return a pointer to the string in memory.
>   */
> @@ -751,7 +737,7 @@ filesize(int f)
>  
>   if (fstat(f, ) >= 0)
>   return (statbuf.st_size);
> - return (seek_filesize(f));
> + return (lseek(f, 0, SEEK_END));
>  }
>  
>  /*

[patch] Remove superfluous seek_filesize() from less

[Jesper Wallin]

Hi all,

I was reading through the code for less/filename.c and noticed that the
calling for seek_filesize() in filesize() is superfluous?  A wild guess
is that it's remains from when BAD_LSEEK was removed?


Jesper Wallin


Index: usr.bin/less/filename.c
===
RCS file: /cvs/src/usr.bin/less/filename.c,v
retrieving revision 1.25
diff -u -p -r1.25 filename.c
--- usr.bin/less/filename.c 17 Sep 2016 15:06:41 -  1.25
+++ usr.bin/less/filename.c 16 Sep 2017 20:28:34 -
@@ -363,20 +363,6 @@ bin_file(int f)
 }
 
 /*
- * Try to determine the size of a file by seeking to the end.
- */
-static off_t
-seek_filesize(int f)
-{
-   off_t spos;
-
-   spos = lseek(f, (off_t)0, SEEK_END);
-   if (spos == (off_t)-1)
-   return (-1);
-   return (spos);
-}
-
-/*
  * Read a string from a file.
  * Return a pointer to the string in memory.
  */
@@ -751,7 +737,7 @@ filesize(int f)
 
if (fstat(f, ) >= 0)
return (statbuf.st_size);
-   return (seek_filesize(f));
+   return (lseek(f, 0, SEEK_END));
 }
 
 /*