FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

GH-120754: Make PY_READ_MAX smaller than max byteobject size by cmaloney · Pull Request #121633 · python/cpython · GitHub

/ cpython Public

GH-120754: Make PY_READ_MAX smaller than max byteobject size #121633

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

Closed
cmaloney wants to merge 4 commits into from
+13 −6
Conversation 2 Commits 4 Checks 0 Files changed 3
Closed

GH-120754: Make PY_READ_MAX smaller than max byteobject size #121633

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5a6e0ea
GH-120754: Make PY_READ_MAX fit in byteobject size
cmaloney Jul 11, 2024
c60ae27
Add news blurb
cmaloney Jul 11, 2024
da8b55c
tweak news
cmaloney Jul 17, 2024
1f4d053
tweak news more
cmaloney Jul 17, 2024
File filter

Filter by extension

Filter by extension .h  (1) .py  (1) .rst  (1) All 3 file types selected
Viewed files
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Unified
Split
Hide whitespace
Diff view
Unified
Split
Hide whitespace
9 changes: 7 additions & 2 deletions Include/internal/pycore_fileutils.h
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,13 @@ extern PyObject* _Py_device_encoding(int);
# define _PY_WRITE_MAX INT_MAX
#else
/* write() should truncate the input to PY_SSIZE_T_MAX bytes,
but it's safer to do it ourself to have a portable behaviour */
# define _PY_READ_MAX PY_SSIZE_T_MAX
but it's safer to do it ourself to have a portable behaviour

read() fills a PyBytes object, which has a capped size defined in
bytesobject.c. Prefer reading less data (meets the API spec) to causing
an overflow error.
*/
# define _PY_READ_MAX PY_SSIZE_T_MAX - 4096
# define _PY_WRITE_MAX PY_SSIZE_T_MAX
#endif

Expand Down
7 changes: 3 additions & 4 deletions Lib/test/test_os.py
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -234,17 +234,16 @@ def test_read(self):
self.assertEqual(s, b"spam")

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

cmaloney Jul 17, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

issue here is that this will result in a memory allocation error on 32 bit machines, and this bigmemtest effectively makes it not run on 32 bit machines (They are unlikely to have that much RAM)
def test_large_read(self, size):
self.addCleanup(os_helper.unlink, os_helper.TESTFN)
create_file(os_helper.TESTFN, b'test')

# Issue #21932: Make sure that os.read() does not raise an
# OverflowError for size larger than INT_MAX
#
# For 32 bit systems, it is expected the read doesn't read all the bytes
# but rather caps to a number it can reasonably read.
with open(os_helper.TESTFN, "rb") as fp:
data = os.read(fp.fileno(), size)

Expand Down
3 changes: 3 additions & 0 deletions Misc/NEWS.d/next/Core and Builtins/2024-07-11-13-57-50.gh-issue-120754.C1HedA.rst
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
``os.read()`` caps read size smaller than the max bytes object size to avoid
getting an OverflowError that 'byte string is too large'. This makes it so the
read call is attempted (although still may fail for other reasons).
Back | FazBrowse Home | New Git URL