From patchwork Mon Dec 1 07:45:37 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Siddhesh Poyarekar X-Patchwork-Id: 4018 Received: (qmail 8145 invoked by alias); 1 Dec 2014 07:45:50 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 8132 invoked by uid 89); 1 Dec 2014 07:45:49 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.3 required=5.0 tests=AWL, BAYES_00, SPF_HELO_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Date: Mon, 1 Dec 2014 13:15:37 +0530 From: Siddhesh Poyarekar To: Andreas Schwab Cc: libc-alpha@sourceware.org, carlos@redhat.com Subject: [PATCH v2] Reset cached offset when reading to end of stream (BZ #17653) Message-ID: <20141201074537.GT24022@spoyarek.pnq.redhat.com> References: <20141127122839.GA31809@spoyarek.pnq.redhat.com> <87sih4vkpg.fsf@igel.home> <20141127150731.GB24022@spoyarek.pnq.redhat.com> <87h9xkveb7.fsf@igel.home> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <87h9xkveb7.fsf@igel.home> User-Agent: Mutt/1.5.22.1-rc1 (2013-10-16) On Thu, Nov 27, 2014 at 04:44:12PM +0100, Andreas Schwab wrote: > For the case of naccbuf != 0 this isn't really EOF, but an error. No > other error case is resetting _offset. Thanks, updated patch below: > Also, what about _IO_wfile_underflow_mmap? All of the _mmap files operate on read-only files, so the only way another active handle can change state (from EOF) is by calling {f,l}seek. In that case, the standard requires that the other handle, upon activation, does an fseek as well, so resetting the cache is not necessary. Siddhesh [BZ #17653] * libio/fileops.c (_IO_new_file_underflow): Unset cached offset on EOF. * libio/wfileops.c (_IO_wfile_underflow): Likewise. * libio/tst-ftell-active-handler.c (fgets_func_t): New type. (fgets_func): Function pointer to fgets and fgetws. (do_ftell_test): Add test to verify ftell value after read EOF. (do_test): Set fgets_func. commit 45bdf7cf9456e69e956f6398a3506d0dc9853338 Author: Siddhesh Poyarekar Date: Thu Nov 27 20:32:57 2014 +0530 Reset cached offset when reading to end of stream (BZ #17653) POSIX allows applications to switch file handles when a read results in an end of file. Unset the cached offset at this point so that it is queried again. diff --git a/libio/fileops.c b/libio/fileops.c index 1fc5719..3ae9682 100644 --- a/libio/fileops.c +++ b/libio/fileops.c @@ -615,7 +615,13 @@ _IO_new_file_underflow (fp) } fp->_IO_read_end += count; if (count == 0) - return EOF; + { + /* If a stream is read to EOF, the calling application may switch active + handles. As a result, our offset cache would no longer be valid, so + unset it. */ + fp->_offset = _IO_pos_BAD; + return EOF; + } if (fp->_offset != _IO_pos_BAD) _IO_pos_adjust (fp->_offset, count); return *(unsigned char *) fp->_IO_read_ptr; diff --git a/libio/tst-ftell-active-handler.c b/libio/tst-ftell-active-handler.c index 72066b4..f69e169 100644 --- a/libio/tst-ftell-active-handler.c +++ b/libio/tst-ftell-active-handler.c @@ -86,7 +86,9 @@ static size_t data_len; static size_t file_len; typedef int (*fputs_func_t) (const void *data, FILE *fp); +typedef void *(*fgets_func_t) (void *ws, int n, FILE *fp); fputs_func_t fputs_func; +fgets_func_t fgets_func; /* This test verifies that the offset reported by ftell is correct after the file is truncated using ftruncate. ftruncate does not change the file @@ -290,20 +292,22 @@ do_ftell_test (const char *filename) int fd_mode; size_t old_off; size_t new_off; + size_t eof_off; } test_modes[] = { /* In w, w+ and r+ modes, the file position should be at the beginning of the file. After the write, the offset should be - updated to data_len. */ - {"w", O_WRONLY | O_TRUNC, 0, data_len}, - {"w+", O_RDWR | O_TRUNC, 0, data_len}, - {"r+", O_RDWR, 0, data_len}, + updated to data_len. We don't use eof_off in w and a modes since + they don't allow reading. */ + {"w", O_WRONLY | O_TRUNC, 0, data_len, 0}, + {"w+", O_RDWR | O_TRUNC, 0, data_len, 2 * data_len}, + {"r+", O_RDWR, 0, data_len, 3 * data_len}, /* 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. 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, 0, 3 * data_len}, + {"a", O_WRONLY, 3 * data_len, 4 * data_len, 5 * data_len}, + {"a+", O_RDWR, 0, 5 * data_len, 6 * data_len}, }; for (int j = 0; j < 2; j++) { @@ -348,12 +352,44 @@ do_ftell_test (const char *filename) if (off != test_modes[i].new_off) { - printf ("Incorrect new offset. Expected %zu but got %ld\n", + printf ("Incorrect new offset. Expected %zu but got %ld", test_modes[i].new_off, off); ret |= 1; } else - printf ("new offset = %ld\n", off); + printf ("new offset = %ld", off); + + /* Read to the end, write some data to the fd and check if ftell can + see the new ofset. Do this test only for files that allow + reading. */ + if (test_modes[i].fd_mode != O_WRONLY) + { + char tmpbuf[data_len]; + + rewind (fp); + + while (fgets_func (tmpbuf, sizeof (tmpbuf), fp) && !feof (fp)); + + write_ret = write (fd, data, data_len); + if (write_ret != data_len) + { + printf ("write failed (%m)\n"); + ret |= 1; + } + off = ftell (fp); + + if (off != test_modes[i].eof_off) + { + printf (", Incorrect offset after read EOF. " + "Expected %zu but got %ld\n", + test_modes[i].eof_off, off); + ret |= 1; + } + else + printf (", offset after EOF = %ld\n", off); + } + else + putc ('\n', stdout); fclose (fp); } @@ -617,6 +653,7 @@ do_test (void) /* Tests for regular files. */ puts ("Regular mode:"); fputs_func = (fputs_func_t) fputs; + fgets_func = (fgets_func_t) fgets; data = char_data; data_len = strlen (char_data); ret |= do_one_test (filename); @@ -638,6 +675,7 @@ do_test (void) return 1; } fputs_func = (fputs_func_t) fputws; + fgets_func = (fgets_func_t) fgetws; data = wide_data; data_len = wcslen (wide_data); ret |= do_one_test (filename); diff --git a/libio/wfileops.c b/libio/wfileops.c index 71281c1..2a003b3 100644 --- a/libio/wfileops.c +++ b/libio/wfileops.c @@ -257,7 +257,10 @@ _IO_wfile_underflow (fp) if (count <= 0) { if (count == 0 && naccbuf == 0) - fp->_flags |= _IO_EOF_SEEN; + { + fp->_flags |= _IO_EOF_SEEN; + fp->_offset = _IO_pos_BAD; + } else fp->_flags |= _IO_ERR_SEEN, count = 0; }