Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(4612)

#26335: Make mmap.write return the number of bytes written like other write methods

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year, 8 months ago by jakub+python.org
Modified:
1 year, 8 months ago
Reviewers:
berker.peksag
CC:
Thomas Wouters, devnull_psf.upfronthosting.co.za, berkerpeksag, Martin Panter, jstasiak
Visibility:
Public.

Patch Set 1 #

Total comments: 4

Patch Set 2 #

Unified diffs Side-by-side diffs Delta from patch set Stats Patch
Doc/library/mmap.rst View 1 1 chunk +6 lines, -1 line 0 comments Download
Lib/test/test_mmap.py View 1 1 chunk +4 lines, -0 lines 0 comments Download
Modules/mmapmodule.c View 1 2 chunks +3 lines, -2 lines 0 comments Download

Messages

Total messages: 1
berkerpeksag
1 year, 8 months ago #1
http://bugs.python.org/review/26335/diff/16558/Doc/library/mmap.rst
File Doc/library/mmap.rst (right):

http://bugs.python.org/review/26335/diff/16558/Doc/library/mmap.rst#newcode266
Doc/library/mmap.rst:266: file pointer and return the number of bytes written
(never less than
We also need a versionchanged directive to explain the change. Something like:

    .. versionchanged:: 3.6
       ``mmapobj.write()`` now returns [...]

http://bugs.python.org/review/26335/diff/16558/Lib/test/test_mmap.py
File Lib/test/test_mmap.py (right):

http://bugs.python.org/review/26335/diff/16558/Lib/test/test_mmap.py#newcode35
Lib/test/test_mmap.py:35: self.assertEqual(f.write(b'\0'* PAGESIZE), PAGESIZE)
I'd prefer to keep existed tests as they are and add a new test instead.
Something like:

    m1 = mmap.mmap(-1, len(data1))
    self.assertEqual(m1.write(b"1"), 1)

http://bugs.python.org/review/26335/diff/16558/Modules/mmapmodule.c
File Modules/mmapmodule.c (left):

http://bugs.python.org/review/26335/diff/16558/Modules/mmapmodule.c#oldcode410
Modules/mmapmodule.c:410: Py_INCREF(Py_None);
This can be removed now.

http://bugs.python.org/review/26335/diff/16558/Modules/mmapmodule.c
File Modules/mmapmodule.c (right):

http://bugs.python.org/review/26335/diff/16558/Modules/mmapmodule.c#newcode411
Modules/mmapmodule.c:411: return PyLong_FromSsize_t(data.len);
We need to save the value of ``data.len`` before the
``PyBuffer_Release(&data);`` call.
Sign in to reply to this message.

RSS Feeds Recent Issues | This issue
This is Rietveld 894c83f36cb7