From fc946851219d3765354a6ee7cdf64b81d1b355df Mon Sep 17 00:00:00 2001 From: nitishch Date: Thu, 14 Dec 2017 09:54:37 +0530 Subject: [PATCH 1/6] bpo-32228 Reset raw_pos after rewinding the stream --- Modules/_io/bufferedio.c | 1 + 1 file changed, 1 insertion(+) diff --git a/Modules/_io/bufferedio.c b/Modules/_io/bufferedio.c index 1ae7a70bbda904..00607484e5c32f 100644 --- a/Modules/_io/bufferedio.c +++ b/Modules/_io/bufferedio.c @@ -838,6 +838,7 @@ buffered_flush_and_rewind_unlocked(buffered *self) the current logical position. */ Py_off_t n; n = _buffered_raw_seek(self, -RAW_OFFSET(self), 1); + self->raw_pos = self->pos; _bufferedreader_reset_buf(self); if (n == -1) return NULL; From 024921f1203ba3d2431a5e755bb11eaf456ef55e Mon Sep 17 00:00:00 2001 From: nitishch Date: Wed, 20 Dec 2017 12:26:34 +0530 Subject: [PATCH 2/6] bpo-32228 Reset write_end value even when write_pos == write_end --- Lib/test/test_fileio.py | 13 +++++++++++++ Modules/_io/bufferedio.c | 3 +-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_fileio.py b/Lib/test/test_fileio.py index 57a02656206ff4..6c402c4e87a2fa 100644 --- a/Lib/test/test_fileio.py +++ b/Lib/test/test_fileio.py @@ -496,6 +496,19 @@ def testTruncate(self): self.assertEqual(f.seek(0, io.SEEK_END), 15) f.close() + f = io.open(TESTFN, 'r+b') + # Fill with some buffer + f.write(b'\x00' * 5000) + f.close() + f = io.open(TESTFN, 'r+b') + f.write(b'\x00' * 4097) + # After write write_pos and write_end are set to 0 + f.read(1) + # read operation makes sure that pos != raw_pos + f.truncate() + self.assertEqual(f.tell(), 4098) + f.close() + def testTruncateOnWindows(self): def bug801631(): # SF bug diff --git a/Modules/_io/bufferedio.c b/Modules/_io/bufferedio.c index d8db758841a612..517ecf4886393a 100644 --- a/Modules/_io/bufferedio.c +++ b/Modules/_io/bufferedio.c @@ -817,7 +817,6 @@ buffered_flush_and_rewind_unlocked(buffered *self) the current logical position. */ Py_off_t n; n = _buffered_raw_seek(self, -RAW_OFFSET(self), 1); - self->raw_pos = self->pos; _bufferedreader_reset_buf(self); if (n == -1) return NULL; @@ -1900,9 +1899,9 @@ _bufferedwriter_flush_unlocked(buffered *self) goto error; } - _bufferedwriter_reset_buf(self); end: + _bufferedwriter_reset_buf(self); Py_RETURN_NONE; error: From 4c9de0ec681241cd37fdb38bad181079e58b558d Mon Sep 17 00:00:00 2001 From: nitishch Date: Fri, 22 Dec 2017 14:53:59 +0530 Subject: [PATCH 3/6] bpo-32228 Removed one unnecessary call and also removed the test temporarily --- Lib/test/test_fileio.py | 13 ------------- Modules/_io/bufferedio.c | 1 - 2 files changed, 14 deletions(-) diff --git a/Lib/test/test_fileio.py b/Lib/test/test_fileio.py index 6c402c4e87a2fa..57a02656206ff4 100644 --- a/Lib/test/test_fileio.py +++ b/Lib/test/test_fileio.py @@ -496,19 +496,6 @@ def testTruncate(self): self.assertEqual(f.seek(0, io.SEEK_END), 15) f.close() - f = io.open(TESTFN, 'r+b') - # Fill with some buffer - f.write(b'\x00' * 5000) - f.close() - f = io.open(TESTFN, 'r+b') - f.write(b'\x00' * 4097) - # After write write_pos and write_end are set to 0 - f.read(1) - # read operation makes sure that pos != raw_pos - f.truncate() - self.assertEqual(f.tell(), 4098) - f.close() - def testTruncateOnWindows(self): def bug801631(): # SF bug diff --git a/Modules/_io/bufferedio.c b/Modules/_io/bufferedio.c index 517ecf4886393a..f2138b16ca1ea8 100644 --- a/Modules/_io/bufferedio.c +++ b/Modules/_io/bufferedio.c @@ -1292,7 +1292,6 @@ _io__Buffered_seek_impl(buffered *self, PyObject *targetobj, int whence) if (res == NULL) goto end; Py_CLEAR(res); - _bufferedwriter_reset_buf(self); } /* TODO: align on block boundary and read buffer if needed? */ From 81fc49936a9a23a721ecc2bcc3b82db7ace5b57b Mon Sep 17 00:00:00 2001 From: nitishch Date: Fri, 22 Dec 2017 16:16:10 +0530 Subject: [PATCH 4/6] bpo-32228 Added other buffer_sizes to the test --- Lib/test/test_io.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/Lib/test/test_io.py b/Lib/test/test_io.py index 9bfe4b0bc6e4be..1b734da235834e 100644 --- a/Lib/test/test_io.py +++ b/Lib/test/test_io.py @@ -1723,6 +1723,21 @@ def test_truncate(self): with self.open(support.TESTFN, "rb", buffering=0) as f: self.assertEqual(f.read(), b"abc") + def test_truncate_after_write(self): + with self.open(support.TESTFN, "wb") as f: + # Fill with some buffer + f.write(b'\x00' * 10000) + buffer_sizes = [8192, 4096, 200] + for buffer_size in buffer_sizes: + with self.open(support.TESTFN, "r+b", buffering=buffer_size) as f: + f.write(b'\x00' * (buffer_size + 1)) + # After write write_pos and write_end are set to 0 + f.read(1) + # read operation makes sure that pos != raw_pos + f.truncate() + self.assertEqual(f.tell(), buffer_size + 2) + + @support.requires_resource('cpu') def test_threads(self): try: From 472eefe6ef44efa56e3a5a18a9412729f48c56a2 Mon Sep 17 00:00:00 2001 From: nitishch Date: Fri, 22 Dec 2017 16:47:51 +0530 Subject: [PATCH 5/6] bpo-32228 Added NEWS entry --- .../NEWS.d/next/Library/2017-12-22-16-47-41.bpo-32228.waPx3q.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Library/2017-12-22-16-47-41.bpo-32228.waPx3q.rst diff --git a/Misc/NEWS.d/next/Library/2017-12-22-16-47-41.bpo-32228.waPx3q.rst b/Misc/NEWS.d/next/Library/2017-12-22-16-47-41.bpo-32228.waPx3q.rst new file mode 100644 index 00000000000000..13b6e524e4d70f --- /dev/null +++ b/Misc/NEWS.d/next/Library/2017-12-22-16-47-41.bpo-32228.waPx3q.rst @@ -0,0 +1 @@ +Reset ``write_end`` value after ``truncate()`` From b83eac024cac4bcf6832f947af68e6bc461b1a94 Mon Sep 17 00:00:00 2001 From: nitishch Date: Fri, 22 Dec 2017 18:29:14 +0530 Subject: [PATCH 6/6] bpo-32228 Added some comments in code and in test files --- Lib/test/test_io.py | 4 +++- .../Library/2017-12-22-16-47-41.bpo-32228.waPx3q.rst | 2 +- Modules/_io/bufferedio.c | 11 +++++++++-- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/Lib/test/test_io.py b/Lib/test/test_io.py index 1b734da235834e..dc1c7c8e72a776 100644 --- a/Lib/test/test_io.py +++ b/Lib/test/test_io.py @@ -1724,6 +1724,9 @@ def test_truncate(self): self.assertEqual(f.read(), b"abc") def test_truncate_after_write(self): + # Ensure that truncate preserves the file position after + # writes longer than the buffer size. + # Issue: https://bugs.python.org/issue32228 with self.open(support.TESTFN, "wb") as f: # Fill with some buffer f.write(b'\x00' * 10000) @@ -1737,7 +1740,6 @@ def test_truncate_after_write(self): f.truncate() self.assertEqual(f.tell(), buffer_size + 2) - @support.requires_resource('cpu') def test_threads(self): try: diff --git a/Misc/NEWS.d/next/Library/2017-12-22-16-47-41.bpo-32228.waPx3q.rst b/Misc/NEWS.d/next/Library/2017-12-22-16-47-41.bpo-32228.waPx3q.rst index 13b6e524e4d70f..3bbe7c495f82d3 100644 --- a/Misc/NEWS.d/next/Library/2017-12-22-16-47-41.bpo-32228.waPx3q.rst +++ b/Misc/NEWS.d/next/Library/2017-12-22-16-47-41.bpo-32228.waPx3q.rst @@ -1 +1 @@ -Reset ``write_end`` value after ``truncate()`` +Ensure that ``truncate()`` preserves the file position (as reported by ``tell()``) after writes longer than the buffer size. diff --git a/Modules/_io/bufferedio.c b/Modules/_io/bufferedio.c index f2138b16ca1ea8..fa6ece8e947451 100644 --- a/Modules/_io/bufferedio.c +++ b/Modules/_io/bufferedio.c @@ -1856,8 +1856,6 @@ _bufferedwriter_raw_write(buffered *self, char *start, Py_ssize_t len) return n; } -/* `restore_pos` is 1 if we need to restore the raw stream position at - the end, 0 otherwise. */ static PyObject * _bufferedwriter_flush_unlocked(buffered *self) { @@ -1900,6 +1898,15 @@ _bufferedwriter_flush_unlocked(buffered *self) end: + /* This ensures that after return from this function, + VALID_WRITE_BUFFER(self) returns false. + + This is a required condition because when a tell() is called + after flushing and if VALID_READ_BUFFER(self) is false, we need + VALID_WRITE_BUFFER(self) to be false to have + RAW_OFFSET(self) == 0. + + Issue: https://bugs.python.org/issue32228 */ _bufferedwriter_reset_buf(self); Py_RETURN_NONE;