This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
On Mon, Mar 17, 2014 at 04:17:46AM -0400, Carlos O'Donell wrote: > Right, so I was thinking along the same lines. If you didn't switch > from write to read then there is no reason why the offset can't > be the end of the file after rewind. However, that subtle change > is a change in our implementation and I agree that it would be best > if we retained the old behaviour. > > Does a `write; rewind; read' work now with the patch? Was the current > patch causing this to fail also or did the `read' start reading at > offset 0? That's not a concern since the read would fail for a file opened in 'a' mode. Only ftell behaviour was relevant here. For 'a+' it reads at offset 0 and that is correct. On 17 March 2014 16:02, Rich Felker <dalias@aerifal.cx> wrote: >> Does this reinstitution of lseek cause any problems for the use of ftell >> on inactive streams? For example is it really correct to have _IO_file_open >> seek to the end of the fully flushed file in append mode? What about >> other users of the fd that might expect the underlying offset to remain >> the same? > > It's perfectly correct for fopen to perform the seek. For fdopen I'm > not so clear; I think it's wrong for fdopen to change the position > except in the case where O_APPEND wasn't set and fdopen adds it (in > this case the implementation can do whatever it wants, no?). I had missed the detail about fdopen not modifying the file descriptor if O_APPEND is already set, thanks for pointing out. I've attached a patch that applies on top of the earlier patch to ensure that fdopen seeks to the end only when O_APPEND is set by fdopen and not when O_APPEND is already set. I have also added a test case to verify this behaviour. None of the old behaviour is affected by this. Siddhesh * libio/iofdopen.c (_IO_new_fdopen): Seek to end only if setting O_APPEND. * libio/tst-ftell-active-handler.c (do_append_test): Add a test case. diff --git a/libio/iofdopen.c b/libio/iofdopen.c index 843a4fa..77a61f1 100644 --- a/libio/iofdopen.c +++ b/libio/iofdopen.c @@ -58,6 +58,7 @@ _IO_new_fdopen (fd, mode) int fd_flags; int i; int use_mmap = 0; + bool do_seek = false; switch (*mode) { @@ -128,6 +129,7 @@ _IO_new_fdopen (fd, mode) */ if ((posix_mode & O_APPEND) && !(fd_flags & O_APPEND)) { + do_seek = true; #ifdef F_SETFL if (_IO_fcntl (fd, F_SETFL, fd_flags | O_APPEND) == -1) #endif @@ -169,8 +171,8 @@ _IO_new_fdopen (fd, mode) /* 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)) + if (do_seek && ((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) diff --git a/libio/tst-ftell-active-handler.c b/libio/tst-ftell-active-handler.c index 40ca58c..e9dc7b3 100644 --- a/libio/tst-ftell-active-handler.c +++ b/libio/tst-ftell-active-handler.c @@ -414,6 +414,61 @@ do_append_test (const char *filename) } } + /* For fdopen in 'a' mode, the file descriptor should not change if the file + is already open with the O_APPEND flag set. */ + fd = open (filename, O_WRONLY | O_APPEND, 0); + if (fd == -1) + { + printf ("open(O_APPEND) failed: %m\n"); + return 1; + } + + off_t seek_ret = lseek (fd, file_len - 1, SEEK_SET); + if (seek_ret == -1) + { + printf ("lseek[O_APPEND][0] failed: %m\n"); + ret |= 1; + } + + fp = fdopen (fd, "a"); + if (fp == NULL) + { + printf ("fdopen(O_APPEND) failed: %m\n"); + close (fd); + return 1; + } + + off_t new_seek_ret = lseek (fd, 0, SEEK_CUR); + if (seek_ret == -1) + { + printf ("lseek[O_APPEND][1] failed: %m\n"); + ret |= 1; + } + + printf ("\tappend: fdopen (file, \"a\"): O_APPEND: "); + + if (seek_ret != new_seek_ret) + { + printf ("incorrectly modified file offset to %ld, should be %ld", + new_seek_ret, seek_ret); + ret |= 1; + } + else + printf ("retained current file offset %ld", seek_ret); + + new_seek_ret = ftello (fp); + + if (seek_ret != new_seek_ret) + { + printf (", ftello reported incorrect offset %ld, should be %ld\n", + new_seek_ret, seek_ret); + ret |= 1; + } + else + printf (", ftello reported correct offset %ld\n", seek_ret); + + fclose (fp); + return ret; }
Attachment:
pgpHmylYJ_TU2.pgp
Description: PGP signature
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |