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>

diff --git a/Doc/library/mmap.rst b/Doc/library/mmap.rst
index 21fa97a2a3cef8e61b946e68f1dc07d4b746fbae..0495bce7add77d9f4e9b6dfeb0ccfd9b3444e660 100644 (file)
--- a/Doc/library/mmap.rst
+++ b/Doc/library/mmap.rst
@@ -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
    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
-   use of the tag parameter will assist in keeping your code portable between
-   Unix and Windows.
+   use of the *tagname* parameter will assist in keeping your code portable
+   between Unix and Windows.
 
    *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*

diff --git a/Lib/test/test_mmap.py b/Lib/test/test_mmap.py
index dfcf3039422af5e0d0328c2b1c5d022a4a3d6a02..1867e8c957f58cf34e5e0f433d8f80ba3a50c759 100644 (file)
--- a/Lib/test/test_mmap.py
+++ b/Lib/test/test_mmap.py
@@ -671,14 +671,16 @@ class MmapTests(unittest.TestCase):
         m2.close()
         m1.close()
 
+        with self.assertRaisesRegex(TypeError, 'tagname'):
+            mmap.mmap(-1, 8, tagname=1)
+
     @cpython_only
     @unittest.skipUnless(os.name == 'nt', 'requires Windows')
     def test_sizeof(self):
         m1 = mmap.mmap(-1, 100)
         tagname = random_tagname()
         m2 = mmap.mmap(-1, 100, tagname=tagname)
-        self.assertEqual(sys.getsizeof(m2),
-                         sys.getsizeof(m1) + len(tagname) + 1)
+        self.assertGreater(sys.getsizeof(m2), sys.getsizeof(m1))
 
     @unittest.skipUnless(os.name == 'nt', 'requires Windows')
     def test_crasher_on_windows(self):

diff --git a/Misc/NEWS.d/next/Windows/2019-06-16-11-27-05.bpo-37308.Iz_NU_.rst b/Misc/NEWS.d/next/Windows/2019-06-16-11-27-05.bpo-37308.Iz_NU_.rst
new file mode 100644 (file)
index 0000000..3ba29d6
--- /dev/null
+++ b/Misc/NEWS.d/next/Windows/2019-06-16-11-27-05.bpo-37308.Iz_NU_.rst
@@ -0,0 +1,2 @@
+Fix mojibake in :class:`mmap.mmap` when using a non-ASCII *tagname* argument
+on Windows.

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