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

Co-authored-by: Erlend E. Aasland <erlend@python.org>
This commit is contained in:
Zackery Spytz 2024-01-11 14:39:47 -08:00 committed by GitHub
parent 2f126a70f3
commit b4d4aa9e8d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 23 additions and 25 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

@ -672,14 +672,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

@ -32,10 +32,6 @@
# include <unistd.h> // close() # include <unistd.h> // close()
#endif #endif
// to support MS_WINDOWS_SYSTEM OpenFileMappingA / CreateFileMappingA
// need to be replaced with OpenFileMappingW / CreateFileMappingW
#if !defined(MS_WINDOWS) || defined(MS_WINDOWS_DESKTOP) || defined(MS_WINDOWS_GAMES)
#ifndef MS_WINDOWS #ifndef MS_WINDOWS
#define UNIX #define UNIX
# ifdef HAVE_FCNTL_H # ifdef HAVE_FCNTL_H
@ -116,7 +112,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
@ -534,7 +530,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 = OpenFileMappingA(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);
@ -563,7 +559,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 = CreateFileMappingA( self->map_handle = CreateFileMappingW(
self->file_handle, self->file_handle,
NULL, NULL,
PAGE_READWRITE, PAGE_READWRITE,
@ -845,7 +841,7 @@ mmap__sizeof__method(mmap_object *self, void *Py_UNUSED(ignored))
{ {
size_t res = _PyObject_SIZE(Py_TYPE(self)); size_t res = _PyObject_SIZE(Py_TYPE(self));
if (self->tagname) { if (self->tagname) {
res += strlen(self->tagname) + 1; res += (wcslen(self->tagname) + 1) * sizeof(self->tagname[0]);
} }
return PyLong_FromSize_t(res); return PyLong_FromSize_t(res);
} }
@ -1400,7 +1396,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 +1406,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 +1539,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,7 +1560,7 @@ 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 = CreateFileMappingA(m_obj->file_handle, m_obj->map_handle = CreateFileMappingW(m_obj->file_handle,
NULL, NULL,
flProtect, flProtect,
size_hi, size_hi,
@ -1771,5 +1769,3 @@ PyInit_mmap(void)
{ {
return PyModuleDef_Init(&mmapmodule); return PyModuleDef_Init(&mmapmodule);
} }
#endif /* !MS_WINDOWS || MS_WINDOWS_DESKTOP || MS_WINDOWS_GAMES */

View File

@ -44,9 +44,7 @@ extern PyObject* PyInit__collections(void);
extern PyObject* PyInit__heapq(void); extern PyObject* PyInit__heapq(void);
extern PyObject* PyInit__bisect(void); extern PyObject* PyInit__bisect(void);
extern PyObject* PyInit__symtable(void); extern PyObject* PyInit__symtable(void);
#if defined(MS_WINDOWS_DESKTOP) || defined(MS_WINDOWS_GAMES)
extern PyObject* PyInit_mmap(void); extern PyObject* PyInit_mmap(void);
#endif
extern PyObject* PyInit__csv(void); extern PyObject* PyInit__csv(void);
extern PyObject* PyInit__sre(void); extern PyObject* PyInit__sre(void);
#if defined(MS_WINDOWS_DESKTOP) || defined(MS_WINDOWS_SYSTEM) || defined(MS_WINDOWS_GAMES) #if defined(MS_WINDOWS_DESKTOP) || defined(MS_WINDOWS_SYSTEM) || defined(MS_WINDOWS_GAMES)