gh-138205: Remove the resize method on mmap object on platforms don't support it #138276
Conversation
There was a problem hiding this comment.
Thank you for your PR, @aisk. Actually, I already had a patch, I just waited until other mmap issues were resolved. It is very similar to your PR, so let's continue with it.
Please update your PR, few new tests that use SystemError were added.
I think it is worth to add an entry in the "Porting to 3.15" section in What's New.
Lib/test/test_mmap.py
Outdated
| @unittest.skipUnless(hasattr(mmap.mmap, 'resize'), 'requires mmap.resize') | ||
| def test_resize(self): | ||
| # Create a file to be mmap'ed. | ||
| f = open(TESTFN, 'bw+') |
There was a problem hiding this comment.
We can simply use with open(...) as f.
Lib/test/test_mmap.py
Outdated
| f = open(TESTFN, 'rb') | ||
| try: |
Lib/test/test_mmap.py
Outdated
| try: | ||
| m.resize(2*mapsize) | ||
| except SystemError: # resize is not universally supported | ||
| except AttributeError: # resize is not universally supported |
There was a problem hiding this comment.
- # Ensuring that readonly mmap can't be resized
- try:
- m.resize(2*mapsize)
- except SystemError: # resize is not universally supported
- pass
- except TypeError:
- pass
- else:
- self.fail("Able to resize readonly memory map")
+ if hasattr(m, 'resize'):
+ # Ensuring that readonly mmap can't be resized
+ with self.assertRaises(TypeError):
+ m.resize(2*mapsize)
Lib/test/test_mmap.py
Outdated
| try: | ||
| m.resize(512) | ||
| except SystemError: | ||
| except AttributeError: |
There was a problem hiding this comment.
Simply use if hasattr(m, 'resize').
- # Try resizing map
- try:
+ if hasattr(m, 'resize'):
m.resize(512)
- except SystemError:
- pass
- else:
- # resize() is supported
Modules/mmapmodule.c
Outdated
| return 0; | ||
|
|
||
| } | ||
| #endif /* defined(MS_WINDOWS) || defined(HAVE_MREMAP) */ |
There was a problem hiding this comment.
We usually write this simpler:
| #endif /* defined(MS_WINDOWS) || defined(HAVE_MREMAP) */ | |
| #endif /* MS_WINDOWS || HAVE_MREMAP */ |
Modules/mmapmodule.c
Outdated
| #endif /* UNIX */ | ||
| } | ||
| } | ||
| #endif /* defined(MS_WINDOWS) || defined(HAVE_MREMAP) */ |
Doc/library/mmap.rst
Outdated
| pagefile) will silently create a new map with the original data copied over | ||
| up to the length of the new size. | ||
|
|
||
| .. availability:: Linux, Windows |
There was a problem hiding this comment.
It is also available on other platforms (e.g. FreeBSD).
See how this is documented for madvise().
aisk
force-pushed
the
remove-mmap-resize
branch
from
91cda99 to
b1241ab
Compare
|
Sorry for the force-push. I messed up the conflict resolution during the merge and had to redo the merge after the push. |
Co-authored-by: Serhiy Storchaka <[email protected]>
Co-authored-by: Victor Stinner <[email protected]>
Co-authored-by: Victor Stinner <[email protected]>
…orms don't support it (python#138276) Co-authored-by: Serhiy Storchaka <[email protected]> Co-authored-by: Victor Stinner <[email protected]>
4 participants
Uh oh!
There was an error while loading. Please reload this page.