gh-138205: Remove the resize method on mmap object on platforms don't support it#138276
Uh oh!
There was an error while loading. Please reload this page.
Conversation
serhiy-storchaka left a comment
serhiy-storchaka left a comment 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 supportedModules/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().
aiskforce-pushed the remove-mmap-resize branch from 91cda99 to b1241abCompare
aisk commented Sep 3, 2025
Sorry for the force-push. I messed up the conflict resolution during the merge and had to redo the merge after the push. |
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Serhiy Storchaka <[email protected]>
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Victor Stinner <[email protected]>
Co-authored-by: Victor Stinner <[email protected]>
kumaraditya303 merged commit c919d02 into python:mainUh oh!
There was an error while loading. Please reload this page.
…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.