8000 gh-129205: Add os.readinto API for reading data into a caller provided buffer by cmaloney · Pull Request #129211 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

gh-129205: Add os.readinto API for reading data into a caller provided buffer #129211

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.

Sign up for GitHub

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

Merged
merged 23 commits into from
Jan 26, 2025
Merged

gh-129205: Add os.readinto API for reading data into a caller provided buffer #129211

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
148d2f7
gh-129205: Add os.readinto API
cmaloney Jan 23, 2025
3b34285
Add blurb
cmaloney Jan 23, 2025
a56f337
Add tests
cmaloney Jan 23, 2025
b75bc9b
Change from :meth: to :func:
cmaloney Jan 23, 2025
992d5f5
Remove length cap, and early exit on negative len.
cmaloney Jan 23, 2025
bbb0e6a
Fix build issues with last commit.
cmaloney Jan 23, 2025
6512788
blurb: Just a literal / neither :func: nor :meth: worked
cmaloney Jan 23, 2025
b4b4c28
Test zero byte long bytearrays, fix typo
cmaloney Jan 23, 2025
4da3fec
Only check set bytes in readinto test
cmaloney Jan 23, 2025
c5f3df0
Add Whats New, os.rst, tweak NEWS
cmaloney Jan 23, 2025
8cc70cd
Update clinic posixmodule.c to be better for help docs
cmaloney Jan 23, 2025
6f8ad36
bytes-like objects to bytes-like object, only one buffer
cmaloney Jan 23, 2025
62f531c
Apply suggestions from code review
cmaloney Jan 23, 2025
c8f5800
Iterate on docs, guarantee no negative return
cmaloney Jan 23, 2025
2457b63
Test non blocking behavior, test more bad args
cmaloney Jan 23, 2025
25125f2
Apply suggestions from code review
cmaloney Jan 24, 2025
52a83fa
Tweak Whats New and NEWS, assert buf->length
cmaloney Jan 24, 2025
a7a0775
clinic
cmaloney Jan 24, 2025
e1aec8e
Tweka os doc
cmaloney Jan 24, 2025
051b0c2
minor doc tweak
cmaloney Jan 24, 2025
9356bc9
Apply suggestions from code review
cmaloney Jan 24, 2025
06e98e8
Sync Doc and help
cmaloney Jan 24, 2025
060a670
Remove bufs, zip from test, just make a local bytearray
cmaloney Jan 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Add tests
  • Loading branch information
@cmaloney
cmaloney committed Jan 23, 2025
commit a56f3370f7ca169ee5596e34e14f7dcb3ab93c95
31 changes: 31 additions & 0 deletions Lib/test/_test_eintr.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,37 @@ def test_read(self):
self.assertEqual(data, os.read(rd, len(data)))
self.assertEqual(proc.wait(), 0)

def test_readinto(self):
rd, wr = os.pipe()
self.addCleanup(os.close, rd)
# wr closed explicitly by parent

# the payload below are smaller than PIPE_BUF, hence the writes will be
# atomic
datas = [b"hello", b"world", b"spam"]
bufs = [bytearray(5), bytearray(5), bytearray(4)]

code = '\n'.join((
'import os, sys, time',
'',
'wr = int(sys.argv[1])',
'datas = %r' % datas,
'sleep_time = %r' % self.sleep_time,
'',
'for data in datas:',
' # let the parent block on read()',
' time.sleep(sleep_time)',
' os.write(wr, data)',
))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
datas = [b"hello", b"world", b"spam"]
bufs = [bytearray(5), bytearray(5), bytearray(4)]
code = '\n'.join((
'import os, sys, time',
'',
'wr = int(sys.argv[1])',
'datas = %r' % datas,
'sleep_time = %r' % self.sleep_time,
'',
'for data in datas:',
' # let the parent block on read()',
' time.sleep(sleep_time)',
' os.write(wr, data)',
))
data = [b"hello", b"world", b"spam"]
bufs = [bytearray(5), bytearray(5), bytearray(4)]
code = '\n'.join((
'import os, sys, time',
'',
'wr = int(sys.argv[1])',
f'data = {data!r}',
f'sleep_time = {self.sleep_time!r}',
'',
'for datum in data:',
' # let the parent block on read()',
' time.sleep(sleep_time)',
' os.write(wr, datum)',
))

Strictly speaking, data is already the plural form of datum.

Copy link
Contributor Author
@cmaloney cmaloney Jan 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For data -> datum the code is copied from test_read just before with minor tweaks; I think the two being very close in code is useful for maintenance more than naming. I think I can get rid of bufs and zip though which will remove the ambiguities / make the code read better


proc = self.subprocess(code, str(wr), pass_fds=[wr])
with kill_on_error(proc):
os.close(wr)
for data, buffer in zip(datas, bufs):
os.readinto(rd, buffer)
self.assertEqual(data, buffer)
self.assertEqual(proc.wait(), 0)

def test_write(self):
rd, wr = os.pipe()
self.addCleanup(os.close, wr)
Expand Down
68 changes: 68 additions & 0 deletions Lib/test/test_os.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,47 @@ def test_read(self):
self.assertEqual(type(s), bytes)
self.assertEqual(s, b"spam")

def test_readinto(self):
with open(os_helper.TESTFN, "w+b") as fobj:
fobj.write(b"spam")
fobj.flush()
fd = fobj.fileno()
os.lseek(fd, 0, 0)
buffer = bytearray(8)
s = os.readinto(fd, buffer)
self.assertEqual(type(s), int)
self.assertEqual(s, 4)
# Should overwrite the first 4 bytes of the buffer.
self.assertEqual(bytes(buffer), b"spam\0\0\0\0")

# Readinto at EOF shold return 0 and not touch buffer
buffer[:] = b"notspam\0"
s = os.readinto(fd, buffer)
self.assertEqual(type(s), int)
self.assertEqual(s, 0)
self.assertEqual(bytes(buffer), b"notspam\0")
s = os.readinto(fd, buffer)
self.assertEqual(s, 0)
self.assertEqual(bytes(buffer), b"notspam\0")

def test_readinto_badbuffer(self):
with open(os_helper.TESTFN, "w+b") as fobj:
fobj.write(b"spam")
fobj.flush()
fd = fobj.fileno()
os.lseek(fd, 0, 0)

for bad_arg in ("test", bytes(), 14):
with self.subTest(f"{type(bad_arg)}"):
with self.assertRaises(TypeError):
os.readinto(fd, bad_arg)

# No data should have been read with the bad arguments.
buffer = bytearray(8)
s = os.readinto(fd, buffer)
self.assertEqual(s, 4)
self.assertEqual(bytes(buffer), b"spam\0\0\0\0")

@support.cpython_only
# Skip the test on 32-bit platforms: the number of bytes must fit in a
# Py_ssize_t type
Expand All @@ -249,6 +290,29 @@ def test_large_read(self, size):
# operating system is free to return less bytes than requested.
self.assertEqual(data, b'test')


@support.cpython_only
# Skip the test on 32-bit platforms: the number of bytes must fit in a
# Py_ssize_t type
@unittest.skipUnless(INT_MAX < PY_SSIZE_T_MAX,
"needs INT_MAX < PY_SSIZE_T_MAX")
@support.bigmemtest(size=INT_MAX + 10, memuse=1, dry_run=False)
def test_large_readinto(self, size):
self.addCleanup(os_helper.unlink, os_helper.TESTFN)
create_file(os_helper.TESTFN, b'test')

# Issue #21932: For readinto the buffer contains the length rather than
# a length being passed explicitly to read, shold still get capped to a
# valid size / not raise an OverflowError for sizes larger than INT_MAX.
buffer = bytearray(INT_MAX+10)
with open(os_helper.TESTFN, "rb") as fp:
length = os.readinto(fp.fileno(), buffer)

# The test does not try to read more than 2 GiB at once because the
# operating system is free to return less bytes than requested.
self.assertEqual(length, 4)
self.assertEqual(buffer[:4], b'test')

def test_write(self):
# os.write() accepts bytes- and buffer-like objects but not strings
fd = os.open(os_helper.TESTFN, os.O_CREAT | os.O_WRONLY)
Expand Down Expand Up @@ -2467,6 +2531,10 @@ def test_lseek(self):
def test_read(self):
self.check(os.read, 1)

@unittest.skipUnless(hasattr(os, 'readinto'), 'test needs os.readinto()')
def test_readinto(self):
self.check(os.readinto, bytearray(5))

@unittest.skipUnless(hasattr(os, 'readv'), 'test needs os.readv()')
def test_readv(self):
buf = bytearray(10)
Expand Down
Loading
0