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
Add _PySys_GetSizeOf() #66389
Comments
Proposed patch adds private function _PySys_GetSizeOf(). It is needed for correct implementing of __sizeof__() methods: bpo-15490, bpo-15513, bpo-15381 (BytesIO.__sizeof__ is broken in 3.5). See discussion about it in bpo-15490. I extracted this patch in separate issue for its simplicity and because it is needed in other issues. |
LGTM |
New changeset b5114747d3ed by Serhiy Storchaka in branch '2.7': New changeset 46504edf7594 by Serhiy Storchaka in branch '3.4': New changeset e23999892e58 by Serhiy Storchaka in branch 'default': |
Thanks Martin. |
The code is missing an overflow check. There should be some verification that __sizeof__ returns a value smaller than SIZE_T_MAX - 24. Also 24 for GC overhead looks strange. IMHO it should be sizeof(PyGC_Head) which can be smaller than 24 bytes. Python 3.5.0a0 (default:601045ceff94, Aug 14 2014, 22:59:49)
[GCC 4.8.2] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> class Overflow:
... def __sizeof__(self):
... return (1<<64)-1
...
>>> sys.getsizeof(Overflow())
23 |
This was discussed in bpo-15490. Such implementation is just incorrect, because we cannot allocate such much memory. |
Here is a patch which adds overflow check. The question is: should sys.getsizeof with the default argument catch OverflowError in additional to TypeError and replace it by default value? |
Christian: I don't see why 24 bytes overhead sounds strange. GC_Head is typedef union _gc_head {
struct {
union _gc_head *gc_next;
union _gc_head *gc_prev;
Py_ssize_t gc_refs;
} gc;
double dummy; /* force worst-case alignment */
} PyGC_Head; which happens to be 3*8=24 bytes on a 64-bit system. |
Revision b5114747d3ed added useless '#ifndef Py_LIMITED_API' check in 2.7 branch. |
Fixed in changeset 2e417d9a2b1c. Thank you Arfrever. |
Here is alternative patch. May be it looks more clear for you. |
New changeset 3537994fa43b by Serhiy Storchaka in branch '2.7': New changeset df5c6b05238e by Serhiy Storchaka in branch '3.4': New changeset b7651f9be4a1 by Serhiy Storchaka in branch 'default': |
Also fixed an error in _PySys_GetSizeOf declaration (thanks yomgui1). Thanks all for reviews and found bugs. |
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: