
This issue trackerhas been migrated toGitHub, and is currentlyread-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.
Created on2016-08-01 07:43 byxiang.zhang, last changed2022-04-11 14:58 byadmin. This issue is nowclosed.
| Files | ||||
|---|---|---|---|---|
| File name | Uploaded | Description | Edit | |
| List_New.patch | xiang.zhang,2016-08-01 07:43 | review | ||
| List_New_Calloc.patch | xiang.zhang,2016-08-04 03:17 | review | ||
| List_New_Calloc_v2.patch | xiang.zhang,2016-08-19 08:42 | review | ||
| Messages (15) | |||
|---|---|---|---|
| msg271774 -(view) | Author: Xiang Zhang (xiang.zhang)*![]() | Date: 2016-08-01 07:43 | |
List_New checks against PY_SIZE_MAX. The upper bound of PyMem_Malloc is PY_SSIZE_T_MAX.Instead of simply changing the constant, another method is delegating overflow check to PyMem_Calloc, so we can avoid the check in unnecessary check in PyMem_Malloc. But I am not sure hiding the check in PyMem_Calloc is a good idea or not. | |||
| msg271798 -(view) | Author: Martin Panter (martin.panter)*![]() | Date: 2016-08-02 01:22 | |
It looks like PyMem_RESIZE() would be a truer equivalent than PyMem_Calloc(), since PyMem_MALLOC() does not initialize the memory. I would be happy with changing to that if you want.PyMem_Malloc() has been limited to PY_SSIZE_T_MAX sinceIssue 2620, although the documentation <https://docs.python.org/3.5/c-api/memory.html#c.PyMem_Malloc> only mentions “size_t”. There is no match for “ssize_t” etc anywhere on that page. | |||
| msg271801 -(view) | Author: Xiang Zhang (xiang.zhang)*![]() | Date: 2016-08-02 04:24 | |
Thanks for your replies.I update the patch with PyMem_Calloc. I think it's better than PyMem_Resize since we need to initialize the memory here, there is a memset(op->ob_item, 0, nbytes) below. | |||
| msg271804 -(view) | Author: Raymond Hettinger (rhettinger)*![]() | Date: 2016-08-02 05:44 | |
Mark, this is your code. Care to comment? | |||
| msg271944 -(view) | Author: Xiang Zhang (xiang.zhang)*![]() | Date: 2016-08-04 03:17 | |
Add another PY_SIZE_MAX replacement I suggest in listobject.c. | |||
| msg273086 -(view) | Author: Mark Dickinson (mark.dickinson)*![]() | Date: 2016-08-19 07:34 | |
The change to use PyMem_Calloc looks good to me. | |||
| msg273088 -(view) | Author: Mark Dickinson (mark.dickinson)*![]() | Date: 2016-08-19 07:43 | |
Also, if you're switching to `PyMem_Calloc`, I'd suggest dropping the assertion at the end, since that's now really an assertion for logic that appears in `PyMem_Calloc` rather than in this function. | |||
| msg273091 -(view) | Author: Xiang Zhang (xiang.zhang)*![]() | Date: 2016-08-19 08:04 | |
Thanks for your reply Mark. But I don't understand your message about the assertion. There is now no assertion in PyList_New. The changed assertion statement is in list_ass_subscript. If this confuses you I am quite sorry to bring in a somewhat unrelated change. :( I just thought it's not worth to open another issue to change that one. Really sorry. | |||
| msg273094 -(view) | Author: Mark Dickinson (mark.dickinson)*![]() | Date: 2016-08-19 08:29 | |
Right, sorry. I'm suggesting dropping that second assertion entirely; essentially, we're moving the responsibility for and knowledge about overflow checking into the PyMem_* functions/macros (which is where it belongs), and I don't see a need to check the equivalent condition in list_ass_subscript.If you do keep the second assertion, you should probably drop the `(size_t)` cast, so that we're not comparing signed with unsigned. | |||
| msg273095 -(view) | Author: Mark Dickinson (mark.dickinson)*![]() | Date: 2016-08-19 08:33 | |
There's also still a use of PY_SIZE_MAX in the list_resize function. Would it be worth fixing that one too? | |||
| msg273096 -(view) | Author: Xiang Zhang (xiang.zhang)*![]() | Date: 2016-08-19 08:42 | |
Just remove it. Regenerate the patch. As for list_resize, I have already fired anotherissue27660. | |||
| msg273097 -(view) | Author: Mark Dickinson (mark.dickinson)*![]() | Date: 2016-08-19 08:46 | |
LGTM. Thanks!I'll apply this later today, unless someone beats me to it. | |||
| msg273277 -(view) | Author: Roundup Robot (python-dev)![]() | Date: 2016-08-21 07:55 | |
New changesetcd3d079ad2b5 by Mark Dickinson in branch 'default':Issue#27662: don't use PY_SIZE_MAX for overflow checking in List_New. Patch by Xiang Zhang.https://hg.python.org/cpython/rev/cd3d079ad2b5 | |||
| msg273278 -(view) | Author: Mark Dickinson (mark.dickinson)*![]() | Date: 2016-08-21 08:00 | |
List_New_Calloc_v2.patch applied. Thanks! | |||
| msg273281 -(view) | Author: Roundup Robot (python-dev)![]() | Date: 2016-08-21 08:31 | |
New changesetc7f9e66826a0 by Mark Dickinson in branch 'default':Issue#27662: add missingMisc/NEWS entry.https://hg.python.org/cpython/rev/c7f9e66826a0 | |||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022-04-11 14:58:34 | admin | set | github: 71849 |
| 2016-08-21 08:31:57 | python-dev | set | messages: +msg273281 |
| 2016-08-21 08:00:51 | mark.dickinson | set | status: open -> closed resolution: fixed messages: +msg273278 stage: patch review -> resolved |
| 2016-08-21 07:55:27 | python-dev | set | nosy: +python-dev messages: +msg273277 |
| 2016-08-19 08:46:20 | mark.dickinson | set | messages: +msg273097 |
| 2016-08-19 08:42:43 | xiang.zhang | set | files: +List_New_Calloc_v2.patch messages: +msg273096 |
| 2016-08-19 08:33:34 | mark.dickinson | set | messages: +msg273095 |
| 2016-08-19 08:29:36 | mark.dickinson | set | messages: +msg273094 |
| 2016-08-19 08:04:53 | xiang.zhang | set | messages: +msg273091 |
| 2016-08-19 07:43:10 | mark.dickinson | set | messages: +msg273088 |
| 2016-08-19 07:34:53 | mark.dickinson | set | messages: +msg273086 |
| 2016-08-04 03:17:07 | xiang.zhang | set | files: +List_New_Calloc.patch messages: +msg271944 |
| 2016-08-04 03:16:22 | xiang.zhang | set | files: -List_New_Calloc.patch |
| 2016-08-02 05:44:19 | rhettinger | set | assignee:mark.dickinson messages: +msg271804 |
| 2016-08-02 04:24:38 | xiang.zhang | set | files: +List_New_Calloc.patch messages: +msg271801 |
| 2016-08-02 01:22:28 | martin.panter | set | stage: patch review messages: +msg271798 versions: - Python 3.5 |
| 2016-08-02 00:36:32 | rhettinger | set | nosy: +mark.dickinson |
| 2016-08-02 00:32:15 | rhettinger | set | messages: -msg271797 |
| 2016-08-02 00:24:31 | rhettinger | set | nosy: +rhettinger,tim.peters messages: +msg271797 |
| 2016-08-01 07:43:10 | xiang.zhang | create | |