This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: add offset to mmap
Type: Stage:
Components: Extension Modules Versions: Python 2.6
process
Status: closed Resolution: accepted
Dependencies: Superseder:
Assigned To: akuchling Nosy List: akuchling, gvanrossum, josiahcarlson, loewis, nnorwitz, phuang, rhettinger, teoliphant, yotam
Priority: normal Keywords: patch

Created on 2003-03-23 14:33 by nnorwitz, last changed 2022-04-10 16:07 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
mmap.diff nnorwitz, 2003-03-23 14:34 patch 1
mmap.diff nnorwitz, 2003-03-29 21:28 patch 2 - with windows support
mmap-test.diff nnorwitz, 2003-04-10 18:20 updated test for windows
mmap-block.diff nnorwitz, 2003-06-27 03:19 minimal patch for excluding block devices from size check
mmap-reg.diff akuchling, 2003-07-14 20:02 Minimal patch for only doing size check on regular files
mmap3.diff loewis, 2006-10-28 18:12 version 3 of patch
mmap.diff phuang, 2007-08-29 14:01
Messages (27)
msg43116 - (view) Author: Neal Norwitz (nnorwitz) * (Python committer) Date: 2003-03-23 14:33
This patch is from Yotam Medini <yotamm at
mellanox.co.il> sent to me in mail.

It adds support for the offset parameter to mmap.

It ignores the check for mmap size "if the file is
character device.  Some device drivers (which I happen
to use) have zero size in fstat buffer, but still one
can seek() read() and tell()."
I added minimal doc and tests.
msg43117 - (view) Author: Neal Norwitz (nnorwitz) * (Python committer) Date: 2003-03-23 15:37
Logged In: YES 
user_id=33168

Email received from Yotam:

I have downloaded and patched the 2.3a source. compiled
locally just this module, and it worked fine for my
application (with offset for character device file) I did
not run the released test though.
msg43118 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2003-03-28 00:12
Logged In: YES 
user_id=21627

I think non-zero offsets need to be supported for Windows as
well.
msg43119 - (view) Author: Neal Norwitz (nnorwitz) * (Python committer) Date: 2003-03-29 21:28
Logged In: YES 
user_id=33168

Sounds fair.  Attached is an updated patch which includes
windows support (I think).  I cannot test on Windows. 
Tested on Linux.  Includes updates for doc, src, and test.
msg43120 - (view) Author: Neal Norwitz (nnorwitz) * (Python committer) Date: 2003-04-09 23:35
Logged In: YES 
user_id=33168

Raymond, could you try to test this patch and see if it
works on Windows?
msg43121 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2003-04-10 16:01
Logged In: YES 
user_id=80475

It doesn't run:

C:\py23\Lib\test>python_d test_mmap.py
Traceback (most recent call last):
  File "test_mmap.py", line 357, in ?
    test_main()
  File "test_mmap.py", line 353, in test_main
    test_offset()
  File "test_mmap.py", line 37, in test_offset
    m = mmap.mmap(f.fileno(), mapsize - PAGESIZE, 
offset=PAGESIZE)
TypeError: 'offset' is an invalid keyword argument for this 
function
[9363 refs]
msg43122 - (view) Author: Neal Norwitz (nnorwitz) * (Python committer) Date: 2003-04-10 17:10
Logged In: YES 
user_id=33168

Hmmm, did Modules/mmapmodule.c get rebuilt?  There is code
in the patch for new_mmap_object() to support the offset
keyword in both Unix & Windows.  I don't see a problem in
the code/patch.
msg43123 - (view) Author: Neal Norwitz (nnorwitz) * (Python committer) Date: 2003-04-10 17:14
Logged In: YES 
user_id=33168

Note to self:  Self, make sure to backport S_ISCHR() fix.
msg43124 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2003-04-10 17:53
Logged In: YES 
user_id=80475

I had posted the wrong traceback (before the rebuild).  
The correct one shows Window Error #87.

Traceback (most recent call last):
  File "test_mmap.py", line 357, in ?
    test_main()
  File "test_mmap.py", line 353, in test_main
    test_offset()
  File "test_mmap.py", line 37, in test_offset
    m = mmap.mmap(f.fileno(), mapsize - PAGESIZE, 
offset=PAGESIZE)
WindowsError: [Errno 87] The parameter is incorrect
[9363 refs]
msg43125 - (view) Author: Neal Norwitz (nnorwitz) * (Python committer) Date: 2003-04-10 18:20
Logged In: YES 
user_id=33168

http://msdn.microsoft.com/library/en-us/fileio/base/mapviewoffile.asp?frame=true
says the offset must be a multiple of the allocation
granularity which used to be hard-coded at 64k.  So maybe if
we made the offset 64k it would work.  The attached patch
makes this test change for windows only.  (I think the Unix
offset must be a multiple of PAGESIZE.)
msg43126 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2003-04-23 03:53
Logged In: YES 
user_id=80475

This is much closer and solves the last problem.
But, it fails test_mmap with a windows errocode  8: not 
enought memory.


Raymond 
msg43127 - (view) Author: Yotam Medini (yotam) * Date: 2003-06-05 14:55
Logged In: YES 
user_id=22624

Just wondering, what is the status of this patch.
I guess it did not make it to 2.2.3.
regards -- yotam
msg43128 - (view) Author: Neal Norwitz (nnorwitz) * (Python committer) Date: 2003-06-05 14:59
Logged In: YES 
user_id=33168

Since this is an enhancement, it will not go into 2.2.x. 
The patch is stuck right now.  It needs to be tested on
Windows, but I can't do that (no Windows development
environment).  If you can update the patch to work on both
Unix and Windows, we can apply the patch.
msg43129 - (view) Author: A.M. Kuchling (akuchling) * (Python committer) Date: 2003-06-26 17:05
Logged In: YES 
user_id=11375

Shouldn't block devices also be excluded from the size check?
msg43130 - (view) Author: Neal Norwitz (nnorwitz) * (Python committer) Date: 2003-06-27 03:19
Logged In: YES 
user_id=33168

Yes, the check for block devices should go in now as a bug
fix I think.  Can anyone test on windows?  I attached a
patch for just this minimal change.
msg43131 - (view) Author: A.M. Kuchling (akuchling) * (Python committer) Date: 2003-07-11 13:54
Logged In: YES 
user_id=11375

There's nothing to test on Windows; the HAVE_FSTAT code is
inside #ifdef UNIX.  If it works on Unix (and it seems to
for me, on Linux), then it should be checked in.
msg43132 - (view) Author: A.M. Kuchling (akuchling) * (Python committer) Date: 2003-07-14 20:02
Logged In: YES 
user_id=11375

According to my reading of the Single Unix Specification
page for sys/stat.h,
st_size only has a sensible value for regular files and for
symlinks.  This implies that the size comparison should only
be done if S_ISREG() returns true.  Patch attached as
mmap_reg.diff; I'll also check it in after running the tests.
msg43133 - (view) Author: A.M. Kuchling (akuchling) * (Python committer) Date: 2003-07-15 13:07
Logged In: YES 
user_id=11375

The device driver check is now committed to CVS, both the
trunk and 2.2 maintenance branch.
msg43134 - (view) Author: Yotam Medini (yotam) * Date: 2003-12-08 19:01
Logged In: YES 
user_id=22624

Recent posts deal mainly about with size checks against
stat() call.
But the main issue here is the support for offset in mmap()ping.
I wonder what is the status of this so 
it would become part of the official release.

If nobody cares to implement and test it for MS-Windows,
let's have it for Linux/Unix only.
msg43135 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2006-10-28 18:12
Logged In: YES 
user_id=21627

I have update the patch (mmap3.diff) for the current trunk
(52498).
I've reviewed and tested the Windows implementation, and found
the original patch to be incorrect: For MapViewOfFile,
instead of
passing the size, offset+size must be passed. I also extended it
to support 64-bit offsets on a 64-bit system (64-bit offsets on
a 32-bit system still aren't supported).

I have doubts about the changes to find and tell: why is it
good that it includes the offset? In particular, for find,
I think it should return the index in the buffer, not the
offset in the file.
msg43136 - (view) Author: Josiah Carlson (josiahcarlson) * (Python triager) Date: 2006-10-28 18:29
Logged In: YES 
user_id=341410

I agree.  In the majority of use-cases that I would
consider, offset into the current mmap object would be more
useful than into the file.  If the mmap has information
about its offset in the file, then the user could easily get
the file offset.
msg43137 - (view) Author: Yotam Medini (yotam) * Date: 2006-10-28 21:46
Logged In: YES 
user_id=22624

I am not sure I am following all arguments.
But a major motivation for having an offset parameter,
is to allow for mapping a segment of a huge file.
A file could have a size which is too large for memory mapping.
But one may need to map just a 'small' segment 
of a well known offset address.

By the way, my experience in this case was in Linux only.
msg43138 - (view) Author: Josiah Carlson (josiahcarlson) * (Python triager) Date: 2006-10-28 21:54
Logged In: YES 
user_id=341410

Right.  The question that Martin had was if you have some
mmap that has been mapped some offset into the large file,
and you do mmap.find(...), do you return an offset relative
to the file on disk, or relative to the mmap?

So if I have m = mmap.mmap(..., startoffset=1024,
length=1024), and I do m.find('X'), do I get some value
-1...1023 inclusive (-1 for not found), or do I get some
value 1024...2047 or -1?  I think it only makes sense to
return -1...1023, so that the return for .find() and other
operations are relative to the mmap, not the file.
msg55420 - (view) Author: Huang Peng (phuang) Date: 2007-08-29 14:01
I updated the patch for trunk (r57652). In this patch, find, move and
flush methods use offset in mmap buffer instead of offset in file. It
also fix a bug in test_mmap.py and modified mmap document. Please review it.
msg56639 - (view) Author: Travis Oliphant (teoliphant) * (Python committer) Date: 2007-10-22 03:01
I think this patch can be committed to SVN.  Paul Moore has tested it on
Windows and I (Travis Oliphant) have tested it on UNIX (Mac OS X).  The
patch also includes documentation updates and tests.
msg56662 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2007-10-22 22:59
Fine with me!
msg56664 - (view) Author: Travis Oliphant (teoliphant) * (Python committer) Date: 2007-10-23 02:44
I applied phuang's patch in revision 58598. This can be closed.
History
Date User Action Args
2022-04-10 16:07:51adminsetgithub: 38205
2007-10-23 06:59:52georg.brandllinkissue1089974 superseder
2007-10-23 03:13:35gvanrossumsetstatus: open -> closed
resolution: accepted
2007-10-23 02:44:11teoliphantsetmessages: + msg56664
2007-10-22 22:59:47gvanrossumsetnosy: + gvanrossum
messages: + msg56662
2007-10-22 03:01:09teoliphantsetnosy: + teoliphant
messages: + msg56639
2007-08-29 14:01:13phuangsetfiles: + mmap.diff
messages: + msg55420
2007-08-28 10:08:23phuangsetnosy: + phuang
2003-03-23 14:33:45nnorwitzcreate