Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions Doc/library/mmap.rst
Original file line number Diff line number Diff line change
Expand Up @@ -191,11 +191,13 @@ To map anonymous memory, -1 should be passed as the fileno along with the length
changes to the given range of bytes will be flushed to disk; otherwise, the
whole extent of the mapping is flushed.

**(Windows version)** A nonzero value returned indicates success; zero
indicates failure.
``None`` is returned to indicate success. An exception is raised when the
call failed.

**(Unix version)** A zero value is returned to indicate success. An
exception is raised when the call failed.
.. versionchanged:: 3.8
Previously, a nonzero value was returned on success; zero was returned
on error under Windows. A zero value was returned on success; an
exception was raised on error under Unix.


.. method:: move(dest, src, count)
Expand Down
7 changes: 7 additions & 0 deletions Doc/whatsnew/3.8.rst
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,13 @@ Changes in the Python API
task name is visible in the ``repr()`` output of :class:`asyncio.Task` and
can also be retrieved using the :meth:`~asyncio.Task.get_name` method.

* The :meth:`mmap.flush() <mmap.mmap.flush>` method now returns ``None`` on
success and raises an exception on error under all platforms. Previously,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be better to accent the main reason of this change. "Previously, the behavior was platform-depended: on success, a nonzero value was returned under Windows and a zero value was returned under Unix, on error, zero was returned under Windows and an exception was raised under Unix." Maybe native speakers can suggest better wording. In any case this PR LGTM.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, I just tweaked it a bit.

its behavior was platform-depended: a nonzero value was returned on success;
zero was returned on error under Windows. A zero value was returned on
success; an exception was raised on error under Unix.
(Contributed by Berker Peksag in :issue:`2122`.)


CPython bytecode changes
------------------------
Expand Down
19 changes: 14 additions & 5 deletions Lib/test/test_mmap.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from test.support import (TESTFN, run_unittest, import_module, unlink,
from test.support import (TESTFN, import_module, unlink,
requires, _2G, _4G, gc_collect, cpython_only)
import unittest
import os
Expand Down Expand Up @@ -741,6 +741,18 @@ def test_concat_repeat_exception(self):
with self.assertRaises(TypeError):
m * 2

def test_flush_return_value(self):
# mm.flush() should return None on success, raise an
# exception on error under all platforms.
mm = mmap.mmap(-1, 16)
self.addCleanup(mm.close)
mm.write(b'python')
result = mm.flush()
self.assertIsNone(result)
if os.name != 'nt':
# 'offset' must be a multiple of mmap.PAGESIZE.
self.assertRaises(OSError, mm.flush, 1, len(b'python'))


class LargeMmapTests(unittest.TestCase):

Expand Down Expand Up @@ -803,8 +815,5 @@ def test_around_4GB(self):
self._test_around_boundary(_4G)


def test_main():
run_unittest(MmapTests, LargeMmapTests)

if __name__ == '__main__':
test_main()
unittest.main()
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
The :meth:`mmap.flush() <mmap.mmap.flush>` method now returns ``None`` on
success, raises an exception on error under all platforms.
11 changes: 7 additions & 4 deletions Modules/mmapmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -569,18 +569,21 @@ mmap_flush_method(mmap_object *self, PyObject *args)
}

if (self->access == ACCESS_READ || self->access == ACCESS_COPY)
return PyLong_FromLong(0);
Py_RETURN_NONE;

#ifdef MS_WINDOWS
return PyLong_FromLong((long) FlushViewOfFile(self->data+offset, size));
if (!FlushViewOfFile(self->data+offset, size)) {
PyErr_SetFromWindowsErr(GetLastError());
return NULL;
}
Py_RETURN_NONE;
#elif defined(UNIX)
/* XXX semantics of return value? */
/* XXX flags for msync? */
if (-1 == msync(self->data + offset, size, MS_SYNC)) {
PyErr_SetFromErrno(PyExc_OSError);
return NULL;
}
return PyLong_FromLong(0);
Py_RETURN_NONE;
#else
PyErr_SetString(PyExc_ValueError, "flush not supported on this system");
return NULL;
Expand Down