2014-03-17 Siddhesh Poyarekar <siddhesh@redhat.com>
+ [BZ #16680]
+ * libio/fileops.c (_IO_file_open): Seek to end of file but
+ don't cache the offset.
+ (get_file_offset): Remove function.
+ (do_ftell): Use cached offset when available.
+ * libio/iofdopen.c (_IO_new_fdopen): Seek to end of file but
+ don't cache the offset.
+ * libio/tst-ftell-active-handler.c (do_rewind_test): New test
+ case.
+ (do_one_test): Call it.
+ (do_ftell_test): Fix up expected old offset for a+ mode.
+ * libio/wfileops.c (do_ftell_wide): Used cached offset when
+ available.
+
* libio/tst-ftell-active-handler.c (do_ftell_test): Don't mix
up test status with function return status.
(do_write_test): Likewise.
* The following bugs are resolved with this release:
15347, 15804, 15894, 16447, 16532, 16545, 16574, 16600, 16609, 16610,
- 16611, 16613, 16623, 16632, 16639, 16642, 16670, 16674, 16677, 16683,
- 16689, 16695, 16701, 16706, 16707.
+ 16611, 16613, 16623, 16632, 16639, 16642, 16670, 16674, 16677, 16680,
+ 16683, 16689, 16695, 16701, 16706, 16707.
* Running the testsuite no longer terminates as soon as a test fails.
Instead, a file tests.sum (xtests.sum from "make xcheck") is generated,
return NULL;
fp->_fileno = fdesc;
_IO_mask_flags (fp, read_write,_IO_NO_READS+_IO_NO_WRITES+_IO_IS_APPENDING);
- if ((read_write & _IO_IS_APPENDING) && (read_write & _IO_NO_READS))
- if (_IO_SEEKOFF (fp, (_IO_off64_t)0, _IO_seek_end, _IOS_INPUT|_IOS_OUTPUT)
- == _IO_pos_BAD && errno != ESPIPE)
- {
- close_not_cancel (fdesc);
- return NULL;
- }
+ /* For append mode, send the file offset to the end of the file. Don't
+ update the offset cache though, since the file handle is not active. */
+ if ((read_write & (_IO_IS_APPENDING | _IO_NO_READS))
+ == (_IO_IS_APPENDING | _IO_NO_READS))
+ {
+ _IO_off64_t new_pos = _IO_SYSSEEK (fp, 0, _IO_seek_end);
+ if (new_pos == _IO_pos_BAD && errno != ESPIPE)
+ {
+ close_not_cancel (fdesc);
+ return NULL;
+ }
+ }
_IO_link_in ((struct _IO_FILE_plus *) fp);
return fp;
}
return 0;
}
-/* Get the current file offset using a system call. This is the safest method
- to get the current file offset, since we are sure that we get the current
- state of the file. Before the stream handle is activated (by using fread,
- fwrite, etc.), an application may alter the state of the file descriptor
- underlying it by calling read/write/lseek on it. Using a cached offset at
- this point will result in returning the incorrect value. Same is the case
- when one switches from reading in a+ mode to writing, where the buffer has
- not been flushed - the cached offset would reflect the reading position
- while the actual write position would be at the end of the file.
-
- do_ftell and do_ftell_wide may resort to using the cached offset in some
- special cases instead of calling get_file_offset, but those cases should be
- thoroughly described. */
-_IO_off64_t
-get_file_offset (_IO_FILE *fp)
-{
- if ((fp->_flags & _IO_IS_APPENDING) == _IO_IS_APPENDING)
- {
- struct stat64 st;
- bool ret = (_IO_SYSSTAT (fp, &st) == 0 && S_ISREG (st.st_mode));
- if (ret)
- return st.st_size;
- else
- return EOF;
- }
- else
- return _IO_SYSSEEK (fp, 0, _IO_seek_cur);
-}
-
-
-/* ftell{,o} implementation. Don't modify any state of the file pointer while
- we try to get the current state of the stream. */
+/* ftell{,o} implementation. The only time we modify the state of the stream
+ is when we have unflushed writes. In that case we seek to the end and
+ record that offset in the stream object. */
static _IO_off64_t
do_ftell (_IO_FILE *fp)
{
- _IO_off64_t result = 0;
- bool use_cached_offset = false;
+ _IO_off64_t result, offset = 0;
/* No point looking at unflushed data if we haven't allocated buffers
yet. */
bool was_writing = (fp->_IO_write_ptr > fp->_IO_write_base
|| _IO_in_put_mode (fp));
+ bool append_mode = (fp->_flags & _IO_IS_APPENDING) == _IO_IS_APPENDING;
+
+ /* When we have unflushed writes in append mode, seek to the end of the
+ file and record that offset. This is the only time we change the file
+ stream state and it is safe since the file handle is active. */
+ if (was_writing && append_mode)
+ {
+ result = _IO_SYSSEEK (fp, 0, _IO_seek_end);
+ if (result == _IO_pos_BAD)
+ return EOF;
+ else
+ fp->_offset = result;
+ }
+
/* Adjust for unflushed data. */
if (!was_writing)
- result -= fp->_IO_read_end - fp->_IO_read_ptr;
+ offset -= 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));
+ offset += fp->_IO_write_ptr - fp->_IO_read_end;
}
- if (use_cached_offset)
- result += fp->_offset;
+ if (fp->_offset != _IO_pos_BAD)
+ result = fp->_offset;
else
- result += get_file_offset (fp);
+ result = _IO_SYSSEEK (fp, 0, _IO_seek_cur);
if (result == EOF)
return result;
+ result += offset;
+
if (result < 0)
{
__set_errno (EINVAL);
return result;
}
-
_IO_off64_t
_IO_new_file_seekoff (fp, offset, dir, mode)
_IO_FILE *fp;
_IO_mask_flags (&new_f->fp.file, read_write,
_IO_NO_READS+_IO_NO_WRITES+_IO_IS_APPENDING);
+ /* For append mode, set the file offset to the end of the file. Don't
+ update the offset cache though, since the file handle is not active. */
+ if ((read_write & (_IO_IS_APPENDING | _IO_NO_READS))
+ == (_IO_IS_APPENDING | _IO_NO_READS))
+ {
+ _IO_off64_t new_pos = _IO_SYSSEEK (&new_f->fp.file, 0, _IO_seek_end);
+ if (new_pos == _IO_pos_BAD && errno != ESPIPE)
+ return NULL;
+ }
return &new_f->fp.file;
}
libc_hidden_ver (_IO_new_fdopen, _IO_fdopen)
typedef int (*fputs_func_t) (const void *data, FILE *fp);
fputs_func_t fputs_func;
+/* Test that ftell output after a rewind is correct. */
+static int
+do_rewind_test (const char *filename)
+{
+ int ret = 0;
+ struct test
+ {
+ const char *mode;
+ int fd_mode;
+ size_t old_off;
+ size_t new_off;
+ } test_modes[] = {
+ {"w", O_WRONLY, 0, data_len},
+ {"w+", O_RDWR, 0, data_len},
+ {"r+", O_RDWR, 0, data_len},
+ /* The new offsets for 'a' and 'a+' modes have to factor in the
+ previous writes since they always append to the end of the
+ file. */
+ {"a", O_WRONLY, 0, 3 * data_len},
+ {"a+", O_RDWR, 0, 4 * data_len},
+ };
+
+ /* Empty the file before the test so that our offsets are simple to
+ calculate. */
+ FILE *fp = fopen (filename, "w");
+ if (fp == NULL)
+ {
+ printf ("Failed to open file for emptying\n");
+ return 1;
+ }
+ fclose (fp);
+
+ for (int j = 0; j < 2; j++)
+ {
+ for (int i = 0; i < sizeof (test_modes) / sizeof (struct test); i++)
+ {
+ FILE *fp;
+ int fd;
+ int fileret;
+
+ printf ("\trewind: %s (file, \"%s\"): ", j == 0 ? "fdopen" : "fopen",
+ test_modes[i].mode);
+
+ if (j == 0)
+ fileret = get_handles_fdopen (filename, fd, fp,
+ test_modes[i].fd_mode,
+ test_modes[i].mode);
+ else
+ fileret = get_handles_fopen (filename, fd, fp, test_modes[i].mode);
+
+ if (fileret != 0)
+ return fileret;
+
+ /* Write some content to the file, rewind and ensure that the ftell
+ output after the rewind is 0. POSIX does not specify what the
+ behavior is when a file is rewound in 'a' mode, so we retain
+ current behavior, which is to keep the 0 offset. */
+ size_t written = fputs_func (data, fp);
+
+ if (written == EOF)
+ {
+ printf ("fputs[1] failed to write data\n");
+ ret |= 1;
+ }
+
+ rewind (fp);
+ long offset = ftell (fp);
+
+ if (offset != test_modes[i].old_off)
+ {
+ printf ("Incorrect old offset. Expected %zu, but got %ld, ",
+ test_modes[i].old_off, offset);
+ ret |= 1;
+ }
+ else
+ printf ("old offset = %ld, ", offset);
+
+ written = fputs_func (data, fp);
+
+ if (written == EOF)
+ {
+ printf ("fputs[1] failed to write data\n");
+ ret |= 1;
+ }
+
+ /* After this write, the offset in append modes should factor in the
+ implicit lseek to the end of file. */
+ offset = ftell (fp);
+ if (offset != test_modes[i].new_off)
+ {
+ printf ("Incorrect new offset. Expected %zu, but got %ld\n",
+ test_modes[i].new_off, offset);
+ ret |= 1;
+ }
+ else
+ printf ("new offset = %ld\n", offset);
+ }
+ }
+ return ret;
+}
+
/* Test that the value of ftell is not cached when the stream handle is not
active. */
static int
{"w", O_WRONLY, 0, data_len},
{"w+", O_RDWR, 0, data_len},
{"r+", O_RDWR, 0, data_len},
- /* For 'a' and 'a+' modes, the initial file position should be the
+ /* For the 'a' mode, the initial file position should be the
current end of file. After the write, the offset has data_len
- added to the old value. */
+ added to the old value. For a+ mode however, the initial file
+ position is the file position of the underlying file descriptor,
+ since it is initially assumed to be in read mode. */
{"a", O_WRONLY, data_len, 2 * data_len},
- {"a+", O_RDWR, 2 * data_len, 3 * data_len},
+ {"a+", O_RDWR, 0, 3 * data_len},
};
for (int j = 0; j < 2; j++)
{
if (off != test_modes[i].new_off)
{
printf ("Incorrect new offset. Expected %zu but got %ld\n",
- test_modes[i].old_off, off);
+ test_modes[i].new_off, off);
ret |= 1;
}
else
ret |= do_ftell_test (filename);
ret |= do_write_test (filename);
ret |= do_append_test (filename);
+ ret |= do_rewind_test (filename);
return ret;
}
}
/* ftell{,o} implementation for wide mode. Don't modify any state of the file
- pointer while we try to get the current state of the stream. */
+ pointer while we try to get the current state of the stream except in one
+ case, which is when we have unflushed writes in append mode. */
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. */
> fp->_wide_data->_IO_write_base)
|| _IO_in_put_mode (fp));
+ bool append_mode = (fp->_flags & _IO_IS_APPENDING) == _IO_IS_APPENDING;
+
+ /* When we have unflushed writes in append mode, seek to the end of the
+ file and record that offset. This is the only time we change the file
+ stream state and it is safe since the file handle is active. */
+ if (was_writing && append_mode)
+ {
+ result = _IO_SYSSEEK (fp, 0, _IO_seek_end);
+ if (result == _IO_pos_BAD)
+ return EOF;
+ else
+ fp->_offset = result;
+ }
+
/* XXX For wide stream with backup store it is not very
reasonable to determine the offset. The pushed-back
character might require a state change and we need not be
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
- 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));
}
- if (use_cached_offset)
+ if (fp->_offset != _IO_pos_BAD)
result = fp->_offset;
else
- result = get_file_offset (fp);
+ result = _IO_SYSSEEK (fp, 0, _IO_seek_cur);
if (result == EOF)
return result;
result += offset;
+ if (result < 0)
+ {
+ __set_errno (EINVAL);
+ return EOF;
+ }
+
return result;
}