gh-142419: Add mmap.set_name method for user custom annotation #142480
Conversation
Modules/mmapmodule.c
Outdated
| } | ||
| #else | ||
| /* naming not supported on this platform */ | ||
| PyErr_SetString(PyExc_ValueError, |
There was a problem hiding this comment.
In ssl.c we usually raise a NotImplementedError in such cases. If you always want to raise the same error, I suggest that you document this ValueError.
| result = m.set_name(long_name) | ||
| self.assertIsNone(result) | ||
|
|
||
| # Test name too long |
There was a problem hiding this comment.
Can you add a test with NUL bytes inside it?
Modules/mmapmodule.c
Outdated
| mmap_mmap_set_name_impl(mmap_object *self, const char *name) | ||
| /*[clinic end generated code: output=1edaf4fd51277760 input=6c7dd91cad205f07]*/ | ||
| { | ||
| const char* prefix = "cpython:mmap:"; |
There was a problem hiding this comment.
| const char* prefix = "cpython:mmap:"; | |
| const char *prefix = "cpython:mmap:"; |
Alternatively you can use a macro and in the printf have MACRO "%s" as a format string instead. Same idea as usages of PRI* macros
| m = mmap.mmap(-1, PAGESIZE) | ||
| self.addCleanup(m.close) | ||
| result = m.set_name('test_mapping') | ||
| self.assertIsNone(result) |
There was a problem hiding this comment.
Would it be possible to add a functional test? Read /proc/pid/maps and check if it contains the string.
There was a problem hiding this comment.
We need to check kernel version and kernel configuration. So it would emit falsy result.
There was a problem hiding this comment.
For the Linux version, you can use @requires_linux_version(x, y, z) decorator. What do you mean by configuration? The feature can be disabled when Linux is configured?
There was a problem hiding this comment.
As I said before this feature is not enabled for Fedora by default even the kernel version is satisfied
There was a problem hiding this comment.
There was a problem hiding this comment.
Do you prefer adding a kernel-configuration checking utility that reads from /proc/config.gz, /proc/config, or /proc/config-$(uname -r)?
There was a problem hiding this comment.
Other thing: Do you know how to inject -X dev while running unittest through CI explicitly?
Co-authored-by: Bénédikt Tran <[email protected]>
Co-authored-by: Bénédikt Tran <[email protected]>
3 participants
📚 Documentation preview 📚: https://cpython-previews--142480.org.readthedocs.build/