diff options
-rw-r--r-- | ChangeLog | 7 | ||||
-rw-r--r-- | libio/fileops.c | 34 | ||||
-rw-r--r-- | libio/iofdopen.c | 16 | ||||
-rw-r--r-- | libio/iofwide.c | 6 | ||||
-rw-r--r-- | libio/wfileops.c | 25 |
5 files changed, 65 insertions, 23 deletions
diff --git a/ChangeLog b/ChangeLog index d8d87d7856..b1ccca4101 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,12 @@ 2014-03-04 Siddhesh Poyarekar <siddhesh@redhat.com> + * libio/fileops.c (do_ftell): Use cached offset when + available. + * libio/iofwide.c (do_ftell_wide): Likewise. + * libio/iofdopen.c (_IO_new_fdopen): Don't use + _IO_file_attach. + * libio/wfileops.c (_IO_fwide): Don't cache offset. + [BZ #16532] * libio/libioP.h (get_file_offset): New function. * libio/fileops.c (get_file_offset): Likewise. diff --git a/libio/fileops.c b/libio/fileops.c index 500629564a..2e7bc8dad9 100644 --- a/libio/fileops.c +++ b/libio/fileops.c @@ -964,12 +964,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. */ @@ -983,8 +979,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 unflushed 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 066ff19000..3f266f7288 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,12 @@ _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 need to record the fd because _IO_file_init will have unset the + offset. It is important to unset the cached offset because the real + offset in the file could change between now and when the handle is + activated and we would then mislead ftell into believing that we have a + valid offset. */ + 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 5cff6325f1..64187e401b 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 7f4c92399f..776bb4a4ba 100644 --- a/libio/wfileops.c +++ b/libio/wfileops.c @@ -602,6 +602,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. */ @@ -702,9 +703,31 @@ do_ftell_wide (_IO_FILE *fp) 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 + unflushed 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; |