gh-81489: Use Unicode APIs for mmap tagname on Windows (GH-14133)

Co-authored-by: Zackery Spytz <zspytz@gmail.com>
Co-authored-by: Erlend E. Aasland <erlend@python.org>
This commit is contained in:
Steve Dower 2024-01-11 23:04:36 +00:00 committed by GitHub
parent d15e1ac828
commit 186c021688
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 32 additions and 27 deletions

View file

@ -62,8 +62,8 @@ To map anonymous memory, -1 should be passed as the fileno along with the length
the same file. If you specify the name of an existing tag, that tag is the same file. If you specify the name of an existing tag, that tag is
opened, otherwise a new tag of this name is created. If this parameter is opened, otherwise a new tag of this name is created. If this parameter is
omitted or ``None``, the mapping is created without a name. Avoiding the omitted or ``None``, the mapping is created without a name. Avoiding the
use of the tag parameter will assist in keeping your code portable between use of the *tagname* parameter will assist in keeping your code portable
Unix and Windows. between Unix and Windows.
*offset* may be specified as a non-negative integer offset. mmap references *offset* may be specified as a non-negative integer offset. mmap references
will be relative to the offset from the beginning of the file. *offset* will be relative to the offset from the beginning of the file. *offset*

View file

@ -671,14 +671,16 @@ class MmapTests(unittest.TestCase):
m2.close() m2.close()
m1.close() m1.close()
with self.assertRaisesRegex(TypeError, 'tagname'):
mmap.mmap(-1, 8, tagname=1)
@cpython_only @cpython_only
@unittest.skipUnless(os.name == 'nt', 'requires Windows') @unittest.skipUnless(os.name == 'nt', 'requires Windows')
def test_sizeof(self): def test_sizeof(self):
m1 = mmap.mmap(-1, 100) m1 = mmap.mmap(-1, 100)
tagname = random_tagname() tagname = random_tagname()
m2 = mmap.mmap(-1, 100, tagname=tagname) m2 = mmap.mmap(-1, 100, tagname=tagname)
self.assertEqual(sys.getsizeof(m2), self.assertGreater(sys.getsizeof(m2), sys.getsizeof(m1))
sys.getsizeof(m1) + len(tagname) + 1)
@unittest.skipUnless(os.name == 'nt', 'requires Windows') @unittest.skipUnless(os.name == 'nt', 'requires Windows')
def test_crasher_on_windows(self): def test_crasher_on_windows(self):

View file

@ -0,0 +1,2 @@
Fix mojibake in :class:`mmap.mmap` when using a non-ASCII *tagname* argument
on Windows.

View file

@ -109,7 +109,7 @@ typedef struct {
#ifdef MS_WINDOWS #ifdef MS_WINDOWS
HANDLE map_handle; HANDLE map_handle;
HANDLE file_handle; HANDLE file_handle;
char * tagname; wchar_t * tagname;
#endif #endif
#ifdef UNIX #ifdef UNIX
@ -539,7 +539,7 @@ mmap_resize_method(mmap_object *self,
CloseHandle(self->map_handle); CloseHandle(self->map_handle);
/* if the file mapping still exists, it cannot be resized. */ /* if the file mapping still exists, it cannot be resized. */
if (self->tagname) { if (self->tagname) {
self->map_handle = OpenFileMapping(FILE_MAP_WRITE, FALSE, self->map_handle = OpenFileMappingW(FILE_MAP_WRITE, FALSE,
self->tagname); self->tagname);
if (self->map_handle) { if (self->map_handle) {
PyErr_SetFromWindowsErr(ERROR_USER_MAPPED_FILE); PyErr_SetFromWindowsErr(ERROR_USER_MAPPED_FILE);
@ -568,7 +568,7 @@ mmap_resize_method(mmap_object *self,
/* create a new file mapping and map a new view */ /* create a new file mapping and map a new view */
/* FIXME: call CreateFileMappingW with wchar_t tagname */ /* FIXME: call CreateFileMappingW with wchar_t tagname */
self->map_handle = CreateFileMapping( self->map_handle = CreateFileMappingW(
self->file_handle, self->file_handle,
NULL, NULL,
PAGE_READWRITE, PAGE_READWRITE,
@ -843,12 +843,11 @@ mmap__repr__method(PyObject *self)
static PyObject * static PyObject *
mmap__sizeof__method(mmap_object *self, void *unused) mmap__sizeof__method(mmap_object *self, void *unused)
{ {
Py_ssize_t res; size_t res = _PyObject_SIZE(Py_TYPE(self));
if (self->tagname) {
res = _PyObject_SIZE(Py_TYPE(self)); res += (wcslen(self->tagname) + 1) * sizeof(self->tagname[0]);
if (self->tagname) }
res += strlen(self->tagname) + 1; return PyLong_FromSize_t(res);
return PyLong_FromSsize_t(res);
} }
#endif #endif
@ -1400,7 +1399,7 @@ new_mmap_object(PyTypeObject *type, PyObject *args, PyObject *kwdict)
DWORD off_lo; /* lower 32 bits of offset */ DWORD off_lo; /* lower 32 bits of offset */
DWORD size_hi; /* upper 32 bits of size */ DWORD size_hi; /* upper 32 bits of size */
DWORD size_lo; /* lower 32 bits of size */ DWORD size_lo; /* lower 32 bits of size */
const char *tagname = ""; PyObject *tagname = Py_None;
DWORD dwErr = 0; DWORD dwErr = 0;
int fileno; int fileno;
HANDLE fh = 0; HANDLE fh = 0;
@ -1410,7 +1409,7 @@ new_mmap_object(PyTypeObject *type, PyObject *args, PyObject *kwdict)
"tagname", "tagname",
"access", "offset", NULL }; "access", "offset", NULL };
if (!PyArg_ParseTupleAndKeywords(args, kwdict, "in|ziL", keywords, if (!PyArg_ParseTupleAndKeywords(args, kwdict, "in|OiL", keywords,
&fileno, &map_size, &fileno, &map_size,
&tagname, &access, &offset)) { &tagname, &access, &offset)) {
return NULL; return NULL;
@ -1543,17 +1542,19 @@ new_mmap_object(PyTypeObject *type, PyObject *args, PyObject *kwdict)
m_obj->weakreflist = NULL; m_obj->weakreflist = NULL;
m_obj->exports = 0; m_obj->exports = 0;
/* set the tag name */ /* set the tag name */
if (tagname != NULL && *tagname != '\0') { if (!Py_IsNone(tagname)) {
m_obj->tagname = PyMem_Malloc(strlen(tagname)+1); if (!PyUnicode_Check(tagname)) {
Py_DECREF(m_obj);
return PyErr_Format(PyExc_TypeError, "expected str or None for "
"'tagname', not %.200s",
Py_TYPE(tagname)->tp_name);
}
m_obj->tagname = PyUnicode_AsWideCharString(tagname, NULL);
if (m_obj->tagname == NULL) { if (m_obj->tagname == NULL) {
PyErr_NoMemory();
Py_DECREF(m_obj); Py_DECREF(m_obj);
return NULL; return NULL;
} }
strcpy(m_obj->tagname, tagname);
} }
else
m_obj->tagname = NULL;
m_obj->access = (access_mode)access; m_obj->access = (access_mode)access;
size_hi = (DWORD)(size >> 32); size_hi = (DWORD)(size >> 32);
@ -1562,12 +1563,12 @@ new_mmap_object(PyTypeObject *type, PyObject *args, PyObject *kwdict)
off_lo = (DWORD)(offset & 0xFFFFFFFF); off_lo = (DWORD)(offset & 0xFFFFFFFF);
/* For files, it would be sufficient to pass 0 as size. /* For files, it would be sufficient to pass 0 as size.
For anonymous maps, we have to pass the size explicitly. */ For anonymous maps, we have to pass the size explicitly. */
m_obj->map_handle = CreateFileMapping(m_obj->file_handle, m_obj->map_handle = CreateFileMappingW(m_obj->file_handle,
NULL, NULL,
flProtect, flProtect,
size_hi, size_hi,
size_lo, size_lo,
m_obj->tagname); m_obj->tagname);
if (m_obj->map_handle != NULL) { if (m_obj->map_handle != NULL) {
m_obj->data = (char *) MapViewOfFile(m_obj->map_handle, m_obj->data = (char *) MapViewOfFile(m_obj->map_handle,
dwDesiredAccess, dwDesiredAccess,