New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
os.read() must use Py_ssize_t for the size parameter #66131
Comments
os.read() currently uses the int type for the size parameter, whereas the C function uses a C size_t parameter:
The Python function must use the Py_ssize_t type, but truncate to INT_MAX on Windows, as done in FileIO.read(). It would help to implement FileIO in pure Python: issue bpo-21859. I don't think that this enhancement should be backported to Python 2.7 nor Python 3.4. |
Patch LGTM. Here is a test for it. |
New changeset e9b401d46e20 by Victor Stinner in branch 'default': |
Thanks for the review.
Your test does not pass because Linux truncates the read() size to 2GB - 4096. I tested with /dev/zero device and with a regular file. I wrote a simplified test to just check that size larger than INT_MAX does not emit an OverflowError. |
New changeset 4a7fcd5273ce by Victor Stinner in branch 'default': |
Failed on 32-bit: ====================================================================== Traceback (most recent call last):
File "/home/serhiy/py/cpython/Lib/test/support/__init__.py", line 1592, in wrapper
return f(self, maxsize)
File "/home/serhiy/py/cpython/Lib/test/test_os.py", line 136, in test_large_read
data = os.read(fp.fileno(), size)
OverflowError: Python int too large to convert to C ssize_t Re-add the @unittest.skipUnless(INT_MAX < PY_SSIZE_T_MAX, "needs INT_MAX < PY_SSIZE_T_MAX") decorator (between cpython_only and bigmemtest). |
New changeset 880e2cdac8b1 by Victor Stinner in branch 'default': |
Oh, I removed it because I thought that it was useless. I didn't understand the purpose of the test. I added a comment. Thanks for the report. |
Thank you for fixing os.read(). |
You're welcome, thanks for your help with the test. I hope that it would FYI I saw EINVAL errors on test_large_read() on some buildbots, Mac OS X |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: