[issue13148] simple bug in mmap size check

2011-10-11 Thread Maxim Yanchenko

Maxim Yanchenko maxim.yanche...@gs.com added the comment:

tried on newer Linux - crashes with my patch.
So I'll be thinking about a workaround, probably a fix for NumPy to avoid using 
mmap in such cases but still provide uniform interface to avoid making special 
conditional handling in all my scripts.
Therefore, I'm no longer pushing for this change, I will need another 
workaround anyway.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13148
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue13148] simple bug in mmap size check

2011-10-11 Thread Maxim Yanchenko

Maxim Yanchenko maxim.yanche...@gs.com added the comment:

 You got lucky, since the offset must be a multiple of the page size.
That's why our header is exactly the page size :)

 Here's what POSIX says
Then it's just another discrepancy between POSIX and Linux, as I received 
ENOMEM instead of EINVAL (RHEL6 on 2.6.32).


Regarding the contradiction, it's probably still worth changing the exception 
message to mmap offset is greater than _or equal to_ file size, to match the 
condition. Just 'greater than' means the '' check, not the '=' check from the 
code, mathematically.

--
resolution: rejected - 

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13148
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue13148] simple bug in mmap size check

2011-10-10 Thread Maxim Yanchenko

New submission from Maxim Yanchenko maxim.yanche...@gs.com:

The condition contradicts the exception text:
if (offset = st.st_size) {
PyErr_SetString(PyExc_ValueError,
mmap offset is greater than file size);
return NULL;
}
The condition should be changed to (offset  st.st_size), similar to the later 
condition which is correct:
} else if (offset + (size_t)map_size  st.st_size) {
PyErr_SetString(PyExc_ValueError,
mmap length is greater than file size);
return NULL;
}

The patch is attached.

--
components: Library (Lib)
files: mmap-greater.patch
keywords: patch
messages: 145319
nosy: jazzer
priority: normal
severity: normal
status: open
title: simple bug in mmap size check
type: behavior
versions: Python 2.7
Added file: http://bugs.python.org/file23372/mmap-greater.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13148
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue13148] simple bug in mmap size check

2011-10-10 Thread Maxim Yanchenko

Maxim Yanchenko maxim.yanche...@gs.com added the comment:

First of all, it doesn't fail (at least on Linux), I tested it before posting.

And the latest, it's not like I'm just stalking around and reading Python 
sources for fun. It's a real and pretty valid case, I hit it while upgrading 
our production code to python 2.7.
I'm using NumPy (linear algebra module) that uses Python's core mmap module 
under the hood.
In NumPy, it's pretty valid to have arrays of size 0.
I have a file with a fixed-size header that holds size of the array and some 
other meta-data. I mmap this file as a NumPy array using the offset equal to 
header size. Of course, if there is no data in the array then the file will be 
just header, and the offset will be equal to the size of the file - here is 
where this bug hits us as I can't load this file with Python 2.7.2 anymore 
(while I was able to with Python 2.5).
This patch fixes this and everything works as expected, giving an array with 
zero dimensions ('shape' in terms of NumPy):
 x.shape
(0,)
 x.size
0

Please kindly consider applying the patch.

--
resolution: invalid - 
status: closed - open

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13148
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue13148] simple bug in mmap size check

2011-10-10 Thread Maxim Yanchenko

Maxim Yanchenko maxim.yanche...@gs.com added the comment:

Well, n+1 is clearly outside the file, wile n is within and therefore valid.

Also, if your position is to forbid zero-size mmapping completely, then the 
checks inside if (map_size == 0) { don't make any sense, especially as they 
may or may fail.
From the existing code, zero-size mmapping is OK as long as offset is OK, so 
the question is whether we consider offset pointing to the end of the file OK. 
To me, it's fine and valid, and there are valid cases like NumPy's zero-size 
arrays, hence the proposed patch.

Removing the check completely is a viable option too, it was already requested 
for special files:
http://bugs.python.org/issue12556

I believe users should have an ability to enjoy whatever their OS provides, and 
deal with the consequences as you said.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13148
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com