[PATCH] Allow fseek optimization also for cookied files.

[PATCH] Allow fseek optimization also for cookied files.

[Giovanni Bajo]

Currently, fseek allows for a seek optimization where it does not
actually seek if the target is within the current buffer. This
optimization, though, is disabled in case of files with custom seek
functions, such as those created by funopen / fopencookie. 

This prevents a useful use case of those functions: creating a transparent
streaming decompressor (eg: gzopen). In fact, it is customary to
allow streaming decompressors to be able to seek forward (by skipping
bytes), but because of the missing seek optimization, even a simple
one-byte seek forward might become a backward seek (that is, a seek
that moves from the end of the current buffer backward to the seek
destination within the buffer).

Since the check was explicit, there might be good reasons for it that
elude me. It looks to me that if buffering is allowed by files
returned by funopen, then the seek optimization cannot be a violation
of the funopen protocol, because otherwise it should also be invalid
to call the underlying readfn callback with size BUFSIZ when the user
requested a different size. People that want an absolute 1:1 match
between user-level calls and callbacks can obtain that by disabling
buffering.

---
 newlib/libc/stdio/fseeko.c     | 8 +++-----
 newlib/libc/stdio64/fseeko64.c | 8 +++-----
 2 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/newlib/libc/stdio/fseeko.c b/newlib/libc/stdio/fseeko.c
index 6fcc8ef06..2dfb4e3db 100644
--- a/newlib/libc/stdio/fseeko.c
+++ b/newlib/libc/stdio/fseeko.c
@@ -185,9 +185,8 @@ _fseeko_r (struct _reent *ptr,
 
   /*
    * Can only optimise if:
-   *	reading (and not reading-and-writing);
-   *	not unbuffered; and
-   *	this is a `regular' Unix file (and hence seekfn==__sseek).
+   *	reading (and not reading-and-writing); and
+   *	not unbuffered.
    * We must check __NBF first, because it is possible to have __NBF
    * and __SOPT both set.
    */
@@ -200,8 +199,7 @@ _fseeko_r (struct _reent *ptr,
     goto dumb;
   if ((fp->_flags & __SOPT) == 0)
     {
-      if (seekfn != __sseek
-	  || fp->_file < 0
+      if (fp->_file < 0
 #ifdef __USE_INTERNAL_STAT64
 	  || _fstat64_r (ptr, fp->_file, &st)
 #else
diff --git a/newlib/libc/stdio64/fseeko64.c b/newlib/libc/stdio64/fseeko64.c
index c5b30aed0..bdee9601f 100644
--- a/newlib/libc/stdio64/fseeko64.c
+++ b/newlib/libc/stdio64/fseeko64.c
@@ -186,9 +186,8 @@ _fseeko64_r (struct _reent *ptr,
 
   /*
    * Can only optimise if:
-   *	reading (and not reading-and-writing);
-   *	not unbuffered; and
-   *	this is a `regular' Unix file (and hence seekfn==__sseek).
+   *	reading (and not reading-and-writing); and
+   *	not unbuffered.
    * We must check __NBF first, because it is possible to have __NBF
    * and __SOPT both set.
    */
@@ -201,8 +200,7 @@ _fseeko64_r (struct _reent *ptr,
     goto dumb;
   if ((fp->_flags & __SOPT) == 0)
     {
-      if (seekfn != __sseek64
-	  || fp->_file < 0
+      if (fp->_file < 0
 	  || _fstat64_r (ptr, fp->_file, &st)
 	  || (st.st_mode & S_IFMT) != S_IFREG)
 	{
-- 
2.39.0

Re: [PATCH] Allow fseek optimization also for cookied files.

[Corinna Vinschen]

On Feb 19 02:40, Giovanni Bajo wrote:
> Currently, fseek allows for a seek optimization where it does not
> actually seek if the target is within the current buffer. This
> optimization, though, is disabled in case of files with custom seek
> functions, such as those created by funopen / fopencookie. 
> 
> This prevents a useful use case of those functions: creating a transparent
> streaming decompressor (eg: gzopen). In fact, it is customary to
> allow streaming decompressors to be able to seek forward (by skipping
> bytes), but because of the missing seek optimization, even a simple
> one-byte seek forward might become a backward seek (that is, a seek
> that moves from the end of the current buffer backward to the seek
> destination within the buffer).
> 
> Since the check was explicit, there might be good reasons for it that
> elude me. It looks to me that if buffering is allowed by files
> returned by funopen, then the seek optimization cannot be a violation
> of the funopen protocol, because otherwise it should also be invalid
> to call the underlying readfn callback with size BUFSIZ when the user
> requested a different size. People that want an absolute 1:1 match
> between user-level calls and callbacks can obtain that by disabling
> buffering.

I'm not entirely sure about the reasing behind this, but the code in
newlib in terms of the seek optimization is still equivalent to the
upstream code in OpenBSD as well as FreeBSD.

Changing such a crucial piece of code and deviating from upstream
sounds like introducing subtil problems...


Corinna