[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

[issue13148] simple bug in mmap size check

2011-10-11 Thread Charles-François Natali
Charles-François Natali neolo...@free.fr added the comment: The condition contradicts the exception text: Why? The offset is zero-based, so 0 = offset size is a valid check. First of all, it doesn't fail (at least on Linux), I tested it before posting. Hmmm. You got lucky, since the

[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

[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;

[issue13148] simple bug in mmap size check

2011-10-10 Thread Antoine Pitrou
Antoine Pitrou pit...@free.fr added the comment: Well, you have to understand what this code does: it tries to prevent non-meaningful offsets. If the offset is equal to the file size, mapping from that offset would not map anything in the file (and the system call may actually fail).

[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

[issue13148] simple bug in mmap size check

2011-10-10 Thread Antoine Pitrou
Antoine Pitrou pit...@free.fr added the comment: I don't think it makes sense to accept mmap'ing empty contents when at offset n but not at offset n + 1. Either we remove the check entirely and let people deal with the consequences, or we keep the check as-is. -- nosy: +neologix

[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