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