Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(95712)

#13390: Hunt memory allocations in addition to reference leaks

Can't Edit
Can't Publish+Mail
Start Review
Created:
7 years, 9 months ago by pitrou
Modified:
6 years, 8 months ago
Reviewers:
ncoghlan
CC:
tim.peters, jcea, Nick Coghlan, AntoinePitrou, haypo, techtonik, Benjamin Peterson, Trundle, skrah, dmalcolm, meadori, Charles-Fran├žois Natali, devnull_psf.upfronthosting.co.za
Visibility:
Public.

Patch Set 1 #

Total comments: 2

Patch Set 2 #

Patch Set 3 #

Patch Set 4 #

Patch Set 5 #

Unified diffs Side-by-side diffs Delta from patch set Stats Patch
Doc/library/sys.rst View 1 2 3 4 1 chunk +11 lines, -0 lines 0 comments Download
Include/objimpl.h View 1 2 3 4 1 chunk +2 lines, -0 lines 0 comments Download
Lib/test/regrtest.py View 1 2 3 4 3 chunks +37 lines, -17 lines 0 comments Download
Lib/test/support.py View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
Lib/test/test_sys.py View 1 2 3 4 2 chunks +24 lines, -0 lines 0 comments Download
Objects/obmalloc.c View 1 2 3 4 5 chunks +20 lines, -1 line 0 comments Download
Python/pythonrun.c View 1 2 3 4 1 chunk +4 lines, -3 lines 0 comments Download
Python/sysmodule.c View 1 2 3 4 2 chunks +15 lines, -0 lines 0 comments Download

Messages

Total messages: 1
Nick Coghlan
7 years, 9 months ago #1
The "sys.getallocatedblocks()" function needs some CPython specific unit tests
in test_sys.

And the name should be getallocatedblocks(), not getallocedblocks()

Other comments are fairly minor - one for consistency with _Py_GetRefTotal(),
the other to make it more obvious to a human reader that the block accounting is
correct when reading the source code.

http://bugs.python.org/review/13390/diff/3633/11366
File Include/objimpl.h (right):

http://bugs.python.org/review/13390/diff/3633/11366#newcode102
Include/objimpl.h:102: PyAPI_DATA(Py_ssize_t) _Py_AllocedBlocks;
Do you have any particular reason for only exposing the data value directly
instead of hiding it behind a function the way _Py_GetRefTotal() does? Either
*works*, of course, but why be inconsistent?

http://bugs.python.org/review/13390/diff/3633/11369
File Objects/obmalloc.c (right):

http://bugs.python.org/review/13390/diff/3633/11369#newcode909
Objects/obmalloc.c:909: return (void *)bp;
To guarantee correct updating of the number of allocated blocks, I suggest
adding an "assert(bp != NULL);" here.
Sign in to reply to this message.

RSS Feeds Recent Issues | This issue
This is Rietveld 894c83f36cb7+