| msg43116 - (view) |
Author: Neal Norwitz (nnorwitz) *  |
Date: 2003-03-23 14:33 |
| This patch is from Yotam Medini 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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) *  |
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) * |
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) * |
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) * |
Date: 2007-10-22 22:59 |
| Fine with me! |
|
|
| msg56664 - (view) |
Author: Travis Oliphant (teoliphant) * |
Date: 2007-10-23 02:44 |
| I applied phuang's patch in revision 58598. This can be closed. |
|
|