bpo-32798: Add restriction on the offset parameter for mmap.flush in the docs #5621

pablogsal · 2018-02-11T15:21:46Z

https://bugs.python.org/issue32798

berkerpeksag · 2018-02-12T20:59:34Z

Doc/library/mmap.rst

@@ -189,7 +189,8 @@ To map anonymous memory, -1 should be passed as the fileno along with the length
      use of this call there is no guarantee that changes are written back before
      the object is destroyed.  If *offset* and *size* are specified, only
      changes to the given range of bytes will be flushed to disk; otherwise, the
-      whole extent of the mapping is flushed.
+      whole extent of the mapping is flushed. *offset* must be a multiple of the
+      PAGESIZE.


Since this is the platform independent documentation, I think it should say "PAGESIZE or ALLOCATIONGRANULARITY".

berkerpeksag · 2018-02-12T20:59:34Z

Doc/library/mmap.rst

@@ -189,7 +189,8 @@ To map anonymous memory, -1 should be passed as the fileno along with the length
      use of this call there is no guarantee that changes are written back before
      the object is destroyed.  If *offset* and *size* are specified, only
      changes to the given range of bytes will be flushed to disk; otherwise, the
-      whole extent of the mapping is flushed.
+      whole extent of the mapping is flushed. *offset* must be a multiple of the


Style nit: Please follow the present style and start the sentence with two spaces.

bedevere-bot · 2018-02-12T20:59:36Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

pablogsal · 2018-02-12T21:03:29Z

I have made the requested changes; please review again

bedevere-bot · 2018-02-12T21:03:31Z

Thanks for making the requested changes!

@berkerpeksag: please review the changes made to this pull request.

serhiy-storchaka · 2018-03-27T14:51:34Z

Doc/library/mmap.rst

@@ -189,7 +189,8 @@ To map anonymous memory, -1 should be passed as the fileno along with the length
      use of this call there is no guarantee that changes are written back before
      the object is destroyed.  If *offset* and *size* are specified, only
      changes to the given range of bytes will be flushed to disk; otherwise, the
-      whole extent of the mapping is flushed.
+      whole extent of the mapping is flushed.  *offset* must be a multiple of the
+      PAGESIZE or ALLOCATIONGRANULARITY.


Please format these constants as

:const:`PAGESIZE` and :const:`ALLOCATIONGRANULARITY`

And in the constructor too.

pablogsal · 2018-03-27T21:28:59Z

I have made the requested changes; please review again

bedevere-bot · 2018-03-27T21:29:01Z

Thanks for making the requested changes!

@berkerpeksag: please review the changes made to this pull request.

pablogsal · 2018-03-27T21:29:18Z

CC: @serhiy-storchaka

serhiy-storchaka · 2018-03-28T09:47:54Z

Doc/library/mmap.rst

@@ -189,7 +189,8 @@ To map anonymous memory, -1 should be passed as the fileno along with the length
      use of this call there is no guarantee that changes are written back before
      the object is destroyed.  If *offset* and *size* are specified, only
      changes to the given range of bytes will be flushed to disk; otherwise, the
-      whole extent of the mapping is flushed.
+      whole extent of the mapping is flushed.  *offset* must be a multiple of the
+      :const:`PAGESIZE` or :const:`ALLOCATIONGRANULARITY`.


Please update also the formatting of other mention of these constants.

zhangyangyu · 2018-03-28T12:48:39Z

FYI, see my comment in the issue, this seems not a necessity required by POSIX. Not sure it matters.

pablogsal · 2018-03-28T14:42:51Z

@zhangyangyu @serhiy-storchaka Should we close the PR then?

zhangyangyu · 2018-03-28T15:07:58Z

I am not sure since there is already such requirement in the doc, like mmap.mmap. Maybe it's already the practical standard? Adding it seems won't hurt.

serhiy-storchaka · 2018-08-24T13:27:18Z

And please update also the formatting of other mention of ALLOCATIONGRANULARITY.

bedevere-bot · 2018-10-20T00:37:57Z

@vstinner: Please replace # with GH- in the commit message next time. Thanks!

miss-islington · 2018-10-20T00:37:57Z

Thanks @pablogsal for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.6, 3.7.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

bedevere-bot · 2018-10-20T00:38:08Z

GH-9989 is a backport of this pull request to the 3.7 branch.

…the docs (pythonGH-5621) Add restriction on the offset parameter for mmap.flush. Explain that ALLOCATIONGRANULARITY is the same as PAGESIZE in Unix. (cherry picked from commit 027664a) Co-authored-by: Pablo Galindo <Pablogsal@gmail.com>

vstinner · 2018-10-20T00:38:12Z

@vstinner: Please replace # with GH- in the commit message next time. Thanks!

Aaaaaaaaaaaaaaaaaaaaaaaaaaaaah.

bedevere-bot · 2018-10-20T00:38:16Z

GH-9990 is a backport of this pull request to the 3.6 branch.

…the docs (pythonGH-5621) Add restriction on the offset parameter for mmap.flush. Explain that ALLOCATIONGRANULARITY is the same as PAGESIZE in Unix. (cherry picked from commit 027664a) Co-authored-by: Pablo Galindo <Pablogsal@gmail.com>

miss-islington · 2018-10-20T00:39:45Z

Thanks @pablogsal for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 2.7.
🐍🍒⛏🤖

bedevere-bot · 2018-10-20T00:39:54Z

GH-9991 is a backport of this pull request to the 2.7 branch.

…the docs (pythonGH-5621) Add restriction on the offset parameter for mmap.flush. Explain that ALLOCATIONGRANULARITY is the same as PAGESIZE in Unix. (cherry picked from commit 027664a) Co-authored-by: Pablo Galindo <Pablogsal@gmail.com>

…the docs (GH-5621) Add restriction on the offset parameter for mmap.flush. Explain that ALLOCATIONGRANULARITY is the same as PAGESIZE in Unix. (cherry picked from commit 027664a) Co-authored-by: Pablo Galindo <Pablogsal@gmail.com>

serhiy-storchaka · 2018-10-20T05:49:52Z

I left this issue open because its author is a core developer now, and is able to merge his PRs yourself (after making last moment changes if needed).

…the docs (python#5621) Add restriction on the offset parameter for mmap.flush. Explain that ALLOCATIONGRANULARITY is the same as PAGESIZE in Unix.

Add restriction on the offset parameter for mmap.flush

a478cb0

the-knights-who-say-ni added the CLA signed label Feb 11, 2018

bedevere-bot added the awaiting review label Feb 11, 2018

berkerpeksag added the skip news label Feb 12, 2018

berkerpeksag requested changes Feb 12, 2018

View changes

bedevere-bot added awaiting changes and removed awaiting review labels Feb 12, 2018

bedevere-bot added awaiting change review and removed awaiting changes labels Feb 12, 2018

berkerpeksag added needs backport to 3.6 needs backport to 3.7 labels Feb 12, 2018

berkerpeksag approved these changes Feb 12, 2018

View changes

bedevere-bot added awaiting merge and removed awaiting change review labels Feb 12, 2018

serhiy-storchaka reviewed Mar 27, 2018

View changes

pablogsal force-pushed the bpo32798 branch from 7c0a07a to bef7628 Compare Mar 27, 2018

bedevere-bot added awaiting change review and removed awaiting merge labels Mar 27, 2018

serhiy-storchaka reviewed Mar 28, 2018

View changes

Correct style

8a65d8a

pablogsal force-pushed the bpo32798 branch from bef7628 to 8a65d8a Compare Mar 28, 2018

fixup! Correct style

2d90d92

pablogsal added 2 commits Aug 24, 2018

Add formatting for ALLOCATIONGRANULARITY occurrences

f334ea4

Explain that ALLOCATIONGRANULARITY is the same as PAGESIZE in Unix

39488c5

serhiy-storchaka approved these changes Aug 27, 2018

View changes

bedevere-bot added awaiting merge and removed awaiting change review labels Aug 27, 2018

vstinner merged commit 027664a into python:master Oct 20, 2018

bedevere-bot removed the awaiting merge label Oct 20, 2018

bedevere-bot removed the needs backport to 3.7 label Oct 20, 2018

bedevere-bot removed the needs backport to 3.6 label Oct 20, 2018

vstinner added the needs backport to 2.7 label Oct 20, 2018

bedevere-bot removed the needs backport to 2.7 label Oct 20, 2018

pablogsal deleted the bpo32798 branch May 19, 2021

bpo-32798: Add restriction on the offset parameter for mmap.flush in the docs #5621

bpo-32798: Add restriction on the offset parameter for mmap.flush in the docs #5621

pablogsal commented Feb 11, 2018 •

edited by bedevere-bot

berkerpeksag Feb 12, 2018

berkerpeksag Feb 12, 2018

bedevere-bot commented Feb 12, 2018

pablogsal commented Feb 12, 2018

bedevere-bot commented Feb 12, 2018

serhiy-storchaka Mar 27, 2018

pablogsal commented Mar 27, 2018

bedevere-bot commented Mar 27, 2018

pablogsal commented Mar 27, 2018

serhiy-storchaka Mar 28, 2018

zhangyangyu commented Mar 28, 2018

pablogsal commented Mar 28, 2018 •

edited

zhangyangyu commented Mar 28, 2018

serhiy-storchaka commented Aug 24, 2018

bedevere-bot commented Oct 20, 2018

miss-islington commented Oct 20, 2018

bedevere-bot commented Oct 20, 2018

vstinner commented Oct 20, 2018

bedevere-bot commented Oct 20, 2018

miss-islington commented Oct 20, 2018

bedevere-bot commented Oct 20, 2018

serhiy-storchaka commented Oct 20, 2018

bpo-32798: Add restriction on the offset parameter for mmap.flush in the docs #5621

bpo-32798: Add restriction on the offset parameter for mmap.flush in the docs #5621

Conversation

pablogsal commented Feb 11, 2018 • edited by bedevere-bot

berkerpeksag Feb 12, 2018

Choose a reason for hiding this comment

berkerpeksag Feb 12, 2018

Choose a reason for hiding this comment

bedevere-bot commented Feb 12, 2018

pablogsal commented Feb 12, 2018

bedevere-bot commented Feb 12, 2018

serhiy-storchaka Mar 27, 2018

Choose a reason for hiding this comment

pablogsal commented Mar 27, 2018

bedevere-bot commented Mar 27, 2018

pablogsal commented Mar 27, 2018

serhiy-storchaka Mar 28, 2018

Choose a reason for hiding this comment

zhangyangyu commented Mar 28, 2018

pablogsal commented Mar 28, 2018 • edited

zhangyangyu commented Mar 28, 2018

serhiy-storchaka commented Aug 24, 2018

bedevere-bot commented Oct 20, 2018

miss-islington commented Oct 20, 2018

bedevere-bot commented Oct 20, 2018

vstinner commented Oct 20, 2018

bedevere-bot commented Oct 20, 2018

miss-islington commented Oct 20, 2018

bedevere-bot commented Oct 20, 2018

serhiy-storchaka commented Oct 20, 2018

pablogsal commented Feb 11, 2018 •

edited by bedevere-bot

pablogsal commented Mar 28, 2018 •

edited