Siddhesh Poyarekar 52f69e7
commit 4a68e8ded4fef2b00fbfc6baf1e79e46389871ca
Siddhesh Poyarekar 52f69e7
Author: Siddhesh Poyarekar <siddhesh@redhat.com>
Siddhesh Poyarekar 52f69e7
Date:   Thu Feb 20 15:55:16 2014 +0530
Siddhesh Poyarekar 52f69e7
Siddhesh Poyarekar 52f69e7
    Use cached offset in ftell when reliable
Siddhesh Poyarekar 52f69e7
    
Siddhesh Poyarekar 52f69e7
    The cached offset is reliable to use in ftell when the stream handle
Siddhesh Poyarekar 52f69e7
    is active.  We can consider a stream as being active when there is
Siddhesh Poyarekar 52f69e7
    unbuffered data.  However, even in this case, we can use the cached
Siddhesh Poyarekar 52f69e7
    offset only when the stream is not being written to in a+ mode,
Siddhesh Poyarekar 52f69e7
    because this case may have unbuffered data and a stale offset; the
Siddhesh Poyarekar 52f69e7
    previous read could have sent it off somewhere other than the end of
Siddhesh Poyarekar 52f69e7
    the file.
Siddhesh Poyarekar 52f69e7
    
Siddhesh Poyarekar 52f69e7
    There were a couple of adjustments necessary to get this to work.
Siddhesh Poyarekar 52f69e7
    Firstly, fdopen now ceases to use _IO_attach_fd because it sets the
Siddhesh Poyarekar 52f69e7
    offset cache to the current file position.  This is not correct
Siddhesh Poyarekar 52f69e7
    because there could be changes to the file descriptor before the
Siddhesh Poyarekar 52f69e7
    stream handle is activated, which would not get reflected.
Siddhesh Poyarekar 52f69e7
    
Siddhesh Poyarekar 52f69e7
    A similar offset caching action is done in _IO_fwide, claiming that
Siddhesh Poyarekar 52f69e7
    wide streams have 'problems' with the file offsets.  There don't seem
Siddhesh Poyarekar 52f69e7
    to be any obvious problems with not having the offset cache available,
Siddhesh Poyarekar 52f69e7
    other than that it will have to be queried in a subsequent
Siddhesh Poyarekar 52f69e7
    read/write/seek.  I have removed this as well.
Siddhesh Poyarekar 52f69e7
    
Siddhesh Poyarekar 52f69e7
    The testsuite passes successfully with these changes on x86_64.
Siddhesh Poyarekar 52f69e7
Siddhesh Poyarekar 52f69e7
diff --git a/libio/fileops.c b/libio/fileops.c
Siddhesh Poyarekar 52f69e7
index a177302..c44a5da 100644
Siddhesh Poyarekar 52f69e7
--- a/libio/fileops.c
Siddhesh Poyarekar 52f69e7
+++ b/libio/fileops.c
Siddhesh Poyarekar 52f69e7
@@ -952,12 +952,8 @@ get_file_offset (_IO_FILE *fp)
Siddhesh Poyarekar 52f69e7
 static _IO_off64_t
Siddhesh Poyarekar 52f69e7
 do_ftell (_IO_FILE *fp)
Siddhesh Poyarekar 52f69e7
 {
Siddhesh Poyarekar 52f69e7
-  _IO_off64_t result;
Siddhesh Poyarekar 52f69e7
-
Siddhesh Poyarekar 52f69e7
-  result = get_file_offset (fp);
Siddhesh Poyarekar 52f69e7
-
Siddhesh Poyarekar 52f69e7
-  if (result == EOF)
Siddhesh Poyarekar 52f69e7
-    return result;
Siddhesh Poyarekar 52f69e7
+  _IO_off64_t result = 0;
Siddhesh Poyarekar 52f69e7
+  bool use_cached_offset = false;
Siddhesh Poyarekar 52f69e7
 
Siddhesh Poyarekar 52f69e7
   /* No point looking at unflushed data if we haven't allocated buffers
Siddhesh Poyarekar 52f69e7
      yet.  */
Siddhesh Poyarekar 52f69e7
@@ -971,8 +967,34 @@ do_ftell (_IO_FILE *fp)
Siddhesh Poyarekar 52f69e7
 	result -= fp->_IO_read_end - fp->_IO_read_ptr;
Siddhesh Poyarekar 52f69e7
       else
Siddhesh Poyarekar 52f69e7
 	result += fp->_IO_write_ptr - fp->_IO_read_end;
Siddhesh Poyarekar 52f69e7
+
Siddhesh Poyarekar 52f69e7
+      /* It is safe to use the cached offset when available if there is
Siddhesh Poyarekar 52f69e7
+	 unbuffered data (indicating that the file handle is active) and the
Siddhesh Poyarekar 52f69e7
+	 handle is not for a file open in a+ mode.  The latter condition is
Siddhesh Poyarekar 52f69e7
+	 because there could be a scenario where there is a switch from read
Siddhesh Poyarekar 52f69e7
+	 mode to write mode using an fseek to an arbitrary position.  In this
Siddhesh Poyarekar 52f69e7
+	 case, there would be unbuffered data due to be appended to the end of
Siddhesh Poyarekar 52f69e7
+	 the file, but the offset may not necessarily be the end of the
Siddhesh Poyarekar 52f69e7
+	 file.  It is fine to use the cached offset when the a+ stream is in
Siddhesh Poyarekar 52f69e7
+	 read mode though, since the offset is maintained correctly in that
Siddhesh Poyarekar 52f69e7
+	 case.  Note that this is not a comprehensive set of cases when the
Siddhesh Poyarekar 52f69e7
+	 offset is reliable.  The offset may be reliable even in some cases
Siddhesh Poyarekar 52f69e7
+	 where there is no buffered input and the handle is active, but it's
Siddhesh Poyarekar 52f69e7
+	 just that we don't have a way to identify that condition reliably.  */
Siddhesh Poyarekar 52f69e7
+      use_cached_offset = (result != 0 && fp->_offset != _IO_pos_BAD
Siddhesh Poyarekar 52f69e7
+			   && ((fp->_flags & (_IO_IS_APPENDING | _IO_NO_READS))
Siddhesh Poyarekar 52f69e7
+			       == (_IO_IS_APPENDING | _IO_NO_READS)
Siddhesh Poyarekar 52f69e7
+			       && was_writing));
Siddhesh Poyarekar 52f69e7
     }
Siddhesh Poyarekar 52f69e7
 
Siddhesh Poyarekar 52f69e7
+  if (use_cached_offset)
Siddhesh Poyarekar 52f69e7
+    result += fp->_offset;
Siddhesh Poyarekar 52f69e7
+  else
Siddhesh Poyarekar 52f69e7
+    result += get_file_offset (fp);
Siddhesh Poyarekar 52f69e7
+
Siddhesh Poyarekar 52f69e7
+  if (result == EOF)
Siddhesh Poyarekar 52f69e7
+    return result;
Siddhesh Poyarekar 52f69e7
+
Siddhesh Poyarekar 52f69e7
   if (result < 0)
Siddhesh Poyarekar 52f69e7
     {
Siddhesh Poyarekar 52f69e7
       __set_errno (EINVAL);
Siddhesh Poyarekar 52f69e7
diff --git a/libio/iofdopen.c b/libio/iofdopen.c
Siddhesh Poyarekar 52f69e7
index 066ff19..4525f0f 100644
Siddhesh Poyarekar 52f69e7
--- a/libio/iofdopen.c
Siddhesh Poyarekar 52f69e7
+++ b/libio/iofdopen.c
Siddhesh Poyarekar 52f69e7
@@ -141,9 +141,6 @@ _IO_new_fdopen (fd, mode)
Siddhesh Poyarekar 52f69e7
 #ifdef _IO_MTSAFE_IO
Siddhesh Poyarekar 52f69e7
   new_f->fp.file._lock = &new_f->lock;
Siddhesh Poyarekar 52f69e7
 #endif
Siddhesh Poyarekar 52f69e7
-  /* Set up initially to use the `maybe_mmap' jump tables rather than using
Siddhesh Poyarekar 52f69e7
-     __fopen_maybe_mmap to do it, because we need them in place before we
Siddhesh Poyarekar 52f69e7
-     call _IO_file_attach or else it will allocate a buffer immediately.  */
Siddhesh Poyarekar 52f69e7
   _IO_no_init (&new_f->fp.file, 0, 0, &new_f->wd,
Siddhesh Poyarekar 52f69e7
 #ifdef _G_HAVE_MMAP
Siddhesh Poyarekar 52f69e7
 	       (use_mmap && (read_write & _IO_NO_WRITES))
Siddhesh Poyarekar 52f69e7
@@ -159,13 +156,10 @@ _IO_new_fdopen (fd, mode)
Siddhesh Poyarekar 52f69e7
 #if  !_IO_UNIFIED_JUMPTABLES
Siddhesh Poyarekar 52f69e7
   new_f->fp.vtable = NULL;
Siddhesh Poyarekar 52f69e7
 #endif
Siddhesh Poyarekar 52f69e7
-  if (_IO_file_attach ((_IO_FILE *) &new_f->fp, fd) == NULL)
Siddhesh Poyarekar 52f69e7
-    {
Siddhesh Poyarekar 52f69e7
-      _IO_setb (&new_f->fp.file, NULL, NULL, 0);
Siddhesh Poyarekar 52f69e7
-      _IO_un_link (&new_f->fp);
Siddhesh Poyarekar 52f69e7
-      free (new_f);
Siddhesh Poyarekar 52f69e7
-      return NULL;
Siddhesh Poyarekar 52f69e7
-    }
Siddhesh Poyarekar 52f69e7
+  /* We only record the fd because _IO_file_init will have unset the offset.
Siddhesh Poyarekar 52f69e7
+     We don't need to get the current offset in the file now since it could
Siddhesh Poyarekar 52f69e7
+     change between now and when the handle is activated.  */
Siddhesh Poyarekar 52f69e7
+  new_f->fp.file._fileno = fd;
Siddhesh Poyarekar 52f69e7
   new_f->fp.file._flags &= ~_IO_DELETE_DONT_CLOSE;
Siddhesh Poyarekar 52f69e7
 
Siddhesh Poyarekar 52f69e7
   _IO_mask_flags (&new_f->fp.file, read_write,
Siddhesh Poyarekar 52f69e7
diff --git a/libio/iofwide.c b/libio/iofwide.c
Siddhesh Poyarekar 52f69e7
index 5cff632..64187e4 100644
Siddhesh Poyarekar 52f69e7
--- a/libio/iofwide.c
Siddhesh Poyarekar 52f69e7
+++ b/libio/iofwide.c
Siddhesh Poyarekar 52f69e7
@@ -199,12 +199,6 @@ _IO_fwide (fp, mode)
Siddhesh Poyarekar 52f69e7
 
Siddhesh Poyarekar 52f69e7
       /* From now on use the wide character callback functions.  */
Siddhesh Poyarekar 52f69e7
       ((struct _IO_FILE_plus *) fp)->vtable = fp->_wide_data->_wide_vtable;
Siddhesh Poyarekar 52f69e7
-
Siddhesh Poyarekar 52f69e7
-      /* One last twist: we get the current stream position.  The wide
Siddhesh Poyarekar 52f69e7
-	 char streams have much more problems with not knowing the
Siddhesh Poyarekar 52f69e7
-	 current position and so we should disable the optimization
Siddhesh Poyarekar 52f69e7
-	 which allows the functions without knowing the position.  */
Siddhesh Poyarekar 52f69e7
-      fp->_offset = _IO_SYSSEEK (fp, 0, _IO_seek_cur);
Siddhesh Poyarekar 52f69e7
     }
Siddhesh Poyarekar 52f69e7
 
Siddhesh Poyarekar 52f69e7
   /* Set the mode now.  */
Siddhesh Poyarekar 52f69e7
diff --git a/libio/wfileops.c b/libio/wfileops.c
Siddhesh Poyarekar 52f69e7
index eda7828..651f5ce 100644
Siddhesh Poyarekar 52f69e7
--- a/libio/wfileops.c
Siddhesh Poyarekar 52f69e7
+++ b/libio/wfileops.c
Siddhesh Poyarekar 52f69e7
@@ -600,6 +600,7 @@ static _IO_off64_t
Siddhesh Poyarekar 52f69e7
 do_ftell_wide (_IO_FILE *fp)
Siddhesh Poyarekar 52f69e7
 {
Siddhesh Poyarekar 52f69e7
   _IO_off64_t result, offset = 0;
Siddhesh Poyarekar 52f69e7
+  bool use_cached_offset = false;
Siddhesh Poyarekar 52f69e7
 
Siddhesh Poyarekar 52f69e7
   /* No point looking for offsets in the buffer if it hasn't even been
Siddhesh Poyarekar 52f69e7
      allocated.  */
Siddhesh Poyarekar 52f69e7
@@ -696,13 +697,36 @@ do_ftell_wide (_IO_FILE *fp)
Siddhesh Poyarekar 52f69e7
 	      offset += outstop - out;
Siddhesh Poyarekar 52f69e7
 	    }
Siddhesh Poyarekar 52f69e7
 
Siddhesh Poyarekar 52f69e7
-	  /* _IO_read_end coincides with fp._offset, so the actual file position
Siddhesh Poyarekar 52f69e7
-	     is fp._offset - (_IO_read_end - new_write_ptr).  */
Siddhesh Poyarekar 52f69e7
+	  /* _IO_read_end coincides with fp._offset, so the actual file
Siddhesh Poyarekar 52f69e7
+	     position is fp._offset - (_IO_read_end - new_write_ptr).  */
Siddhesh Poyarekar 52f69e7
 	  offset -= fp->_IO_read_end - fp->_IO_write_ptr;
Siddhesh Poyarekar 52f69e7
+
Siddhesh Poyarekar 52f69e7
 	}
Siddhesh Poyarekar 52f69e7
+
Siddhesh Poyarekar 52f69e7
+      /* It is safe to use the cached offset when available if there is
Siddhesh Poyarekar 52f69e7
+	 unbuffered data (indicating that the file handle is active) and
Siddhesh Poyarekar 52f69e7
+	 the handle is not for a file open in a+ mode.  The latter
Siddhesh Poyarekar 52f69e7
+	 condition is because there could be a scenario where there is a
Siddhesh Poyarekar 52f69e7
+	 switch from read mode to write mode using an fseek to an arbitrary
Siddhesh Poyarekar 52f69e7
+	 position.  In this case, there would be unbuffered data due to be
Siddhesh Poyarekar 52f69e7
+	 appended to the end of the file, but the offset may not
Siddhesh Poyarekar 52f69e7
+	 necessarily be the end of the file.  It is fine to use the cached
Siddhesh Poyarekar 52f69e7
+	 offset when the a+ stream is in read mode though, since the offset
Siddhesh Poyarekar 52f69e7
+	 is maintained correctly in that case.  Note that this is not a
Siddhesh Poyarekar 52f69e7
+	 comprehensive set of cases when the offset is reliable.  The
Siddhesh Poyarekar 52f69e7
+	 offset may be reliable even in some cases where there is no
Siddhesh Poyarekar 52f69e7
+	 buffered input and the handle is active, but it's just that we
Siddhesh Poyarekar 52f69e7
+	 don't have a way to identify that condition reliably.  */
Siddhesh Poyarekar 52f69e7
+      use_cached_offset = (offset != 0 && fp->_offset != _IO_pos_BAD
Siddhesh Poyarekar 52f69e7
+			   && ((fp->_flags & (_IO_IS_APPENDING | _IO_NO_READS))
Siddhesh Poyarekar 52f69e7
+			       == (_IO_IS_APPENDING | _IO_NO_READS)
Siddhesh Poyarekar 52f69e7
+			       && was_writing));
Siddhesh Poyarekar 52f69e7
     }
Siddhesh Poyarekar 52f69e7
 
Siddhesh Poyarekar 52f69e7
-  result = get_file_offset (fp);
Siddhesh Poyarekar 52f69e7
+  if (use_cached_offset)
Siddhesh Poyarekar 52f69e7
+    result = fp->_offset;
Siddhesh Poyarekar 52f69e7
+  else
Siddhesh Poyarekar 52f69e7
+    result = get_file_offset (fp);
Siddhesh Poyarekar 52f69e7
 
Siddhesh Poyarekar 52f69e7
   if (result == EOF)
Siddhesh Poyarekar 52f69e7
     return result;