os.read() must use Py_ssize_t for the size parameter

[vstinner]

BPO	21932
Nosy	@vstinner, @serhiy-storchaka
Files	posix_read_py_ssize_t.patch test_posix_read_py_ssize_t.patch

^{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:

assignee = None
closed_at = <Date 2014-07-11.15:12:57.459>
created_at = <Date 2014-07-07.13:37:54.769>
labels = ['type-feature']
title = 'os.read() must use Py_ssize_t for the size parameter'
updated_at = <Date 2014-07-12.15:27:41.398>
user = 'https://github.com/vstinner'

bugs.python.org fields:

activity = <Date 2014-07-12.15:27:41.398>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2014-07-11.15:12:57.459>
closer = 'vstinner'
components = []
creation = <Date 2014-07-07.13:37:54.769>
creator = 'vstinner'
dependencies = []
files = ['35882', '35884']
hgrepos = []
issue_num = 21932
keywords = ['patch']
message_count = 10.0
messages = ['222462', '222465', '222756', '222757', '222760', '222828', '222831', '222832', '222836', '222845']
nosy_count = 3.0
nosy_names = ['vstinner', 'python-dev', 'serhiy.storchaka']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = None
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue21932'
versions = ['Python 3.5']

[vstinner]

os.read() currently uses the int type for the size parameter, whereas the C function uses a C size_t parameter:

   ssize_t read(int fd, void *buf, size_t count);

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.

[serhiy-storchaka]

Patch LGTM. Here is a test for it.

[python-dev]

New changeset e9b401d46e20 by Victor Stinner in branch 'default':
Issue bpo-21932: os.read() now uses a :c:func:`Py_ssize_t` type instead of
http://hg.python.org/cpython/rev/e9b401d46e20

[vstinner]

Patch LGTM.

Thanks for the review.

Here is a test for it.

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.

[python-dev]

New changeset 4a7fcd5273ce by Victor Stinner in branch 'default':
Issue bpo-21932: Ooops, os.read(fd, size) allocates a buffer of size bytes, even
http://hg.python.org/cpython/rev/4a7fcd5273ce

[serhiy-storchaka]

Failed on 32-bit:

======================================================================
ERROR: test_large_read (test.test_os.FileTests)
----------------------------------------------------------------------

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).

[python-dev]

New changeset 880e2cdac8b1 by Victor Stinner in branch 'default':
Issue bpo-21932: Skip test_os.test_large_read() on 32-bit system
http://hg.python.org/cpython/rev/880e2cdac8b1

[vstinner]

Re-add the @unittest.skipUnless(INT_MAX < PY_SSIZE_T_MAX, "needs INT_MAX < PY_SSIZE_T_MAX") decorator (between cpython_only and bigmemtest).

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.

[serhiy-storchaka]

Thank you for fixing os.read().

[vstinner]

Serhiy Storchaka added the comment:

Thank you for fixing os.read().

You're welcome, thanks for your help with the test. I hope that it would
help you to implement FileIO in Python.

FYI I saw EINVAL errors on test_large_read() on some buildbots, Mac OS X
and FreeBSD if I remember correctly, when I forgot the bigmem decorator.
And FreeBSD kills the process if there is not enough memory.