-
-
Notifications
You must be signed in to change notification settings - Fork 33.3k
gh-138205: Remove the resize method on mmap object on platforms don't support it #138276
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 6 commits
e3c9987
2faa469
a251dea
95ce088
42e562e
e08de1c
ea35d4b
2413ceb
b1241ab
1d36a44
590523f
0ad3c26
e4ab905
b44d044
a73e936
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -57,6 +57,7 @@ def test_basic(self): | |
| f.write(b'\0'* (PAGESIZE-3) ) | ||
| f.flush() | ||
| m = mmap.mmap(f.fileno(), 2 * PAGESIZE) | ||
| self.addCleanup(m.close) | ||
| finally: | ||
| f.close() | ||
|
|
||
|
|
@@ -114,31 +115,34 @@ def test_basic(self): | |
| # Try to seek to negative position... | ||
| self.assertRaises(ValueError, m.seek, -len(m)-1, 2) | ||
|
|
||
| # Try resizing map | ||
| @unittest.skipUnless(hasattr(mmap.mmap, 'resize'), 'requires mmap.resize') | ||
| def test_resize(self): | ||
| # Create a file to be mmap'ed. | ||
| f = open(TESTFN, 'bw+') | ||
|
||
| try: | ||
| m.resize(512) | ||
| except SystemError: | ||
| # resize() not supported | ||
| # No messages are printed, since the output of this test suite | ||
| # would then be different across platforms. | ||
| pass | ||
| else: | ||
| # resize() is supported | ||
| self.assertEqual(len(m), 512) | ||
| # Check that we can no longer seek beyond the new size. | ||
| self.assertRaises(ValueError, m.seek, 513, 0) | ||
|
|
||
| # Check that the underlying file is truncated too | ||
| # (bug #728515) | ||
| f = open(TESTFN, 'rb') | ||
| try: | ||
| f.seek(0, 2) | ||
| self.assertEqual(f.tell(), 512) | ||
| finally: | ||
| f.close() | ||
| self.assertEqual(m.size(), 512) | ||
| # Write 2 pages worth of data to the file | ||
| f.write(b'\0'* 2 * PAGESIZE) | ||
| f.flush() | ||
| m = mmap.mmap(f.fileno(), 2 * PAGESIZE) | ||
| self.addCleanup(m.close) | ||
| finally: | ||
| f.close() | ||
|
|
||
| m.close() | ||
| # Try resizing map | ||
| m.resize(512) | ||
| self.assertEqual(len(m), 512) | ||
| # Check that we can no longer seek beyond the new size. | ||
| self.assertRaises(ValueError, m.seek, 513, 0) | ||
|
|
||
| # Check that the underlying file is truncated too | ||
| # (bug #728515) | ||
| f = open(TESTFN, 'rb') | ||
| try: | ||
|
||
| f.seek(0, 2) | ||
| self.assertEqual(f.tell(), 512) | ||
| finally: | ||
| f.close() | ||
| self.assertEqual(m.size(), 512) | ||
|
|
||
| def test_access_parameter(self): | ||
| # Test for "access" keyword parameter | ||
|
|
@@ -186,7 +190,7 @@ def test_access_parameter(self): | |
| # Ensuring that readonly mmap can't be resized | ||
| try: | ||
| m.resize(2*mapsize) | ||
| except SystemError: # resize is not universally supported | ||
| except AttributeError: # resize is not universally supported | ||
|
||
| pass | ||
| except TypeError: | ||
| pass | ||
|
|
@@ -242,8 +246,9 @@ def test_access_parameter(self): | |
| with open(TESTFN, "rb") as fp: | ||
| self.assertEqual(fp.read(), b'c'*mapsize, | ||
| "Copy-on-write test data file should not be modified.") | ||
| # Ensuring copy-on-write maps cannot be resized | ||
| self.assertRaises(TypeError, m.resize, 2*mapsize) | ||
| if hasattr(m, 'resize'): | ||
| # Ensuring copy-on-write maps cannot be resized | ||
| self.assertRaises(TypeError, m.resize, 2*mapsize) | ||
aisk marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| m.close() | ||
|
|
||
| # Ensuring invalid access parameter raises exception | ||
|
|
@@ -285,10 +290,11 @@ def test_trackfd_parameter(self): | |
| with self.assertRaises(OSError) as err_cm: | ||
| m.size() | ||
| self.assertEqual(err_cm.exception.errno, errno.EBADF) | ||
| with self.assertRaises(ValueError): | ||
| m.resize(size * 2) | ||
| with self.assertRaises(ValueError): | ||
| m.resize(size // 2) | ||
| if hasattr(m, 'resize'): | ||
| with self.assertRaises(ValueError): | ||
| m.resize(size * 2) | ||
| with self.assertRaises(ValueError): | ||
| m.resize(size // 2) | ||
| self.assertEqual(m.closed, False) | ||
|
|
||
| # Smoke-test other API | ||
|
|
@@ -311,8 +317,9 @@ def test_trackfd_neg1(self): | |
| with mmap.mmap(-1, size, trackfd=False) as m: | ||
| with self.assertRaises(OSError): | ||
| m.size() | ||
| with self.assertRaises(ValueError): | ||
| m.resize(size // 2) | ||
| if hasattr(m, 'resize'): | ||
| with self.assertRaises(ValueError): | ||
| m.resize(size // 2) | ||
| self.assertEqual(len(m), size) | ||
| m[0] = ord('a') | ||
| assert m[0] == ord('a') | ||
|
|
@@ -617,7 +624,7 @@ def test_offset (self): | |
| # Try resizing map | ||
| try: | ||
| m.resize(512) | ||
| except SystemError: | ||
| except AttributeError: | ||
|
||
| pass | ||
| else: | ||
| # resize() is supported | ||
|
|
@@ -812,14 +819,12 @@ def test_write_returning_the_number_of_bytes_written(self): | |
| self.assertEqual(mm.write(b"yz"), 2) | ||
| self.assertEqual(mm.write(b"python"), 6) | ||
|
|
||
| @unittest.skipUnless(hasattr(mmap.mmap, 'resize'), 'requires mmap.resize') | ||
| def test_resize_past_pos(self): | ||
| m = mmap.mmap(-1, 8192) | ||
| self.addCleanup(m.close) | ||
| m.read(5000) | ||
| try: | ||
| m.resize(4096) | ||
| except SystemError: | ||
| self.skipTest("resizing not supported") | ||
| m.resize(4096) | ||
| self.assertEqual(m.read(14), b'') | ||
| self.assertRaises(ValueError, m.read_byte) | ||
| self.assertRaises(ValueError, m.write_byte, 42) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| Removed the :meth:`~mmap.mmap.resize` method on platforms that don't support the | ||
| underlying syscall, instead of raising a :exc:`SystemError`. |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -634,6 +634,7 @@ is_writable(mmap_object *self) | |||||
| return 0; | ||||||
| } | ||||||
|
|
||||||
| #if defined(MS_WINDOWS) || defined(HAVE_MREMAP) | ||||||
| static int | ||||||
| is_resizeable(mmap_object *self) | ||||||
| { | ||||||
|
|
@@ -656,6 +657,7 @@ is_resizeable(mmap_object *self) | |||||
| return 0; | ||||||
|
|
||||||
| } | ||||||
| #endif /* defined(MS_WINDOWS) || defined(HAVE_MREMAP) */ | ||||||
|
||||||
| #endif /* defined(MS_WINDOWS) || defined(HAVE_MREMAP) */ | |
| #endif /* MS_WINDOWS || HAVE_MREMAP */ |
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is also available on other platforms (e.g. FreeBSD).
See how this is documented for
madvise().