Repository navigation
bpo-24658: Fix read/write on file with a size greater than 2GB on OSX #1705
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
Changes from 16 commits
f810129
1b7d412
3c447d8
b5b2754
1130c44
207f00f
7064bd5
84b09f9
a8c0aa1
3d7fe38
bb3e082
fc08e60
17d0d04
468dbbe
4140591
86fa808
83dd759
15e176d
d4ec7ec
a21bfcb
4382256
a696899
b8d1f24
3e887f5
530799a
4c9a61f
04ccd69
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -81,6 +81,18 @@ PyAPI_FUNC(int) _Py_EncodeLocaleEx( | |
| #ifndef Py_LIMITED_API | ||
| PyAPI_FUNC(PyObject *) _Py_device_encoding(int); | ||
|
|
||
| #if defined(MS_WINDOWS) || defined(__APPLE__) | ||
| /* On Windows, the count parameter of read() is an int. | ||
| * macOS fails when reading or writing more than 2GB, see bpo-24658 */ | ||
| # define _PY_READ_MAX INT_MAX | ||
| # define _PY_WRITE_MAX INT_MAX | ||
| #else | ||
| # define _PY_READ_MAX PY_SSIZE_T_MAX | ||
| # define _PY_WRITE_MAX PY_SSIZE_T_MAX | ||
| /* write() should truncate count to PY_SSIZE_T_MAX, but it's safer | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nitpick: you may align /* on define :-) and move the comment before # define _PY_WRITE_MAX, not after. |
||
| * to do it ourself to have a portable behaviour */ | ||
| #endif | ||
|
|
||
| #ifdef MS_WINDOWS | ||
| struct _Py_stat_struct { | ||
| unsigned long st_dev; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,12 +5,12 @@ | |
| import stat | ||
| import sys | ||
| import unittest | ||
| from test.support import TESTFN, requires, unlink | ||
| from test.support import TESTFN, requires, unlink, bigmemtest, _2G | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. bigmmemtest is now unused, please remove it.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yep, thanks for your feedback
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. note for myself: bigmemtest is now used. |
||
| import io # C implementation of io | ||
| import _pyio as pyio # Python implementation of io | ||
|
|
||
| # size of file to create (>2 GiB; 2 GiB == 2,147,483,648 bytes) | ||
| size = 2500000000 | ||
| size = 2_500_000_000 | ||
|
|
||
| class LargeFileTest: | ||
| """Test that each file function works as expected for large | ||
|
|
@@ -45,6 +45,13 @@ def tearDownClass(cls): | |
| raise cls.failureException('File was not truncated by opening ' | ||
| 'with mode "wb"') | ||
|
|
||
| @bigmemtest(size=size+1, memuse=1) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add Use |
||
| def test_large_read(self, _size): | ||
| # bpo-24658: Test that a read greater than 2GB does not fail. | ||
| with self.open(TESTFN, "rb") as f: | ||
| self.assertEqual(len(f.read()), size + 1) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed |
||
| self.assertEqual(f.tell(), size + 1) | ||
|
|
||
| def test_osstat(self): | ||
| self.assertEqual(os.stat(TESTFN)[stat.ST_SIZE], size+1) | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Fix read/write on file with a size > 2GB on macOS | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. rather than ">", I would prefer to write: On macOS, fix reading from and writing into a file with a size larger than 2 GiB. |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -791,11 +791,9 @@ _io_FileIO_read_impl(fileio *self, Py_ssize_t size) | |
| if (size < 0) | ||
| return _io_FileIO_readall_impl(self); | ||
|
|
||
| #ifdef MS_WINDOWS | ||
| /* On Windows, the count parameter of read() is an int */ | ||
| if (size > INT_MAX) | ||
| size = INT_MAX; | ||
| #endif | ||
| if (size > _PY_READ_MAX) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure that it's ok to use this if on OS other than macOS and Windows, since size type is ssize_t. I expect "condition is always false" warnings on many recent compilers. I would prefer to keep an #ifdef.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. note for myself: the issue has been fixed |
||
| size = _PY_READ_MAX; | ||
| } | ||
|
|
||
| bytes = PyBytes_FromStringAndSize(NULL, size); | ||
| if (bytes == NULL) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick: you may align /* on define :-)