8000 bpo-37732: Fix GCC warning in _PyObject_Malloc() by vstinner · Pull Request #15333 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

bpo-37732: Fix GCC warning in _PyObject_Malloc() #15333

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

Jump to bottom
Merged
merged 2 commits into from
Aug 20, 2019
Merged

bpo-37732: Fix GCC warning in _PyObject_Malloc() #15333

merged 2 commits into from
Aug 20, 2019

Conversation

vstinner
Copy link
Member
@vstinner vstinner commented Aug 19, 2019 •
edited by bedevere-bot
Loading

pymalloc_alloc() now returns directly the pointer, return NULL on
memory allocation error.

allocate_from_new_pool() already uses NULL as marker for "allocation
failed".

https://bugs.python.org/issue37732

@vstinner
bpo-37732: Fix GCC warning in _PyObject_Malloc()
a0113f0 
pymalloc_alloc() now returns directly the pointer, return NULL on
memory allocation error.

allocate_from_new_pool() already uses NULL as marker for "allocation
failed".
@vstinner vstinner requested a review from methane August 19, 2019 10:27
@vstinner
Copy link
Member Author
vstinner commented Aug 19, 2019 •
edited by bedevere-bot
Loading

PR #15309 proposes a different fix: always initialize ptr to NULL. This change is only needed to make GCC warning quiet, whereas ptr is always initialized.

My change rewrites the code to avoid the warning without adding an useless initialization.

Note: I wrote the pymalloc_alloc(void *ctx, void **ptr_p, size_t nbytes)helper function as part of a larger refactoring change, commit 9ed83c4:

commit 9ed83c40855b57c10988f76770a4eb825e034cd8
Author: Victor Stinner <victor.stinner@gmail.com>
Date:   Tue Oct 31 12:18:10 2017 -0700

    [bpo-18835](https://bugs.python.org/issue18835): Cleanup pymalloc (#4200)
    
    Cleanup pymalloc:
    
    * Rename _PyObject_Alloc() to pymalloc_alloc()
    * Rename _PyObject_FreeImpl() to pymalloc_free()
    * Rename _PyObject_Realloc() to pymalloc_realloc()
    * pymalloc_alloc() and pymalloc_realloc() don't fallback on the raw
      allocator anymore, it now must be done by the caller
    * Add "success" and "failed" labels to pymalloc_alloc() and
      pymalloc_free()
    * pymalloc_alloc() and pymalloc_free() don't update
      num_allocated_blocks anymore: it should be done in the caller
    * _PyObject_Calloc() is now responsible to fill the memory block
      allocated by pymalloc with zeros
    * Simplify pymalloc_alloc() prototype
    * _PyObject_Realloc() now calls _PyObject_Malloc() rather than
      calling directly pymalloc_alloc()
    
    _PyMem_DebugRawAlloc() and _PyMem_DebugRawRealloc():
    
    * document the layout of a memory block
    * don't increase the serial number if the allocation failed
    * check for integer overflow before computing the total size
    * add a 'data' variable to make the code easiler to follow
    
    test_setallocators() of _testcapimodule.c now test also the context.

@vstinner vstinner mentioned this pull request Aug 19, 2019
Closed
serhiy-storchaka
serhiy-storchaka approved these changes Aug 19, 2019
View reviewed changes
Copy link
Member
@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

LGTM, but one check is redundant now.

qigangxu
qigangxu reviewed Aug 20, 2019
View reviewed changes
Objects/obmalloc.c
if (LIKELY(pool != pool->nextpool)) {
/*
* There is a used pool for this size class.
* Pick up the head block of its free list.
*/
++pool->ref.count;
bp = pool->freeblock;
assert(bp != NULL);

if (UNLIKELY((pool->freeblock = *(block **)bp) == NULL)) {
// Reached the end of the free list, try to extend it.
Copy link
Contributor

Choose a reason for hiding this comment

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

It's better to use comment style "/* */" instead of "//" here，to ensure consistent context.

Suggested change
// Reached the end of the free list, try to extend it.
/* Reached the end of the free list, try to extend it.
*/

Copy link
Member Author

Choose a reason for hiding this comment

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

This code was likely written before PEP 7 was updated to allow a subset of C99. My PR doesn't add these comments ;-)

@vstinner vstinner merged commit 18f8dcf into python:master Aug 20, 2019
@vstinner vstinner deleted the pymalloc_alloc_warn branch August 20, 2019 11:28
@miss-islington
Copy link
Contributor

Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Sorry, @vstinner, I could not cleanly backport this to 3.7 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 18f8dcfa10d8a858b152d12a9ad8fa83b7e967f0 3.7

@miss-islington
< 8000 span data-view-component="true"> Copy link
Contributor

Sorry @vstinner, I had trouble checking out the 3.8 backport branch.
Please backport using cherry_picker on command line.
cherry_picker 18f8dcfa10d8a858b152d12a9ad8fa83b7e967f0 3.8

@serhiy-storchaka
Copy link
Member

👍

@bedevere-bot
Copy link

GH-15342 is a backport of this pull request to the 3.8 branch.

vstinner added a commit that referenced this pull request Aug 20, 2019
@vstinner
bpo-37732: Fix GCC warning in _PyObject_Malloc() (GH-15333) (GH-15342)
8000 30e5aff 
pymalloc_alloc() now returns directly the pointer, return NULL on
memory allocation error.

allocate_from_new_pool() already uses NULL as marker for "allocation
failed".

(cherry picked from commit 18f8dcf)
vstinner added a commit that referenced this pull request Aug 20, 2019
@vstinner
bpo-37732: Fix GCC warning in _PyObject_Malloc() (GH-15333) (GH-15342) (
c9a484a 
GH-15343)

pymalloc_alloc() now returns directly the pointer, return NULL on
memory allocation error.

allocate_from_new_pool() already uses NULL as marker for "allocation
failed".

(cherry picked from commit 18f8dcf)
(cherry picked from commit 30e5aff)
lisroach pushed a commit to lisroach/cpython that referenced this pull request Sep 10, 2019
@vstinner @lisroach
bpo-37732: Fix GCC warning in _PyObject_Malloc() (pythonGH-15333)
c6a45e8 
pymalloc_alloc() now returns directly the pointer, return NULL on
memory allocation error.

allocate_from_new_pool() already uses NULL as marker for "allocation
failed".
DinoV pushed a commit to DinoV/cpython that referenced this pull request Jan 14, 2020
@vstinner @DinoV
bpo-37732: Fix GCC warning in _PyObject_Malloc() (pythonGH-15333)
e6362ce 
pymalloc_alloc() now returns directly the pointer, return NULL on
memory allocation error.

allocate_from_new_pool() already uses NULL as marker for "allocation
failed".
websurfer5 pushed a commit to websurfer5/cpython that referenced this pull request Jul 20, 2020
@vstinner @websurfer5
bpo-37732: Fix GCC warning in _PyObject_Malloc() (pythonGH-15333)
d6d53e5 
pymalloc_alloc() now returns directly the pointer, return NULL on
memory allocation error.

allocate_from_new_pool() already uses NULL as marker for "allocation
failed".
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@qigangxu qigangxu qigangxu left review comments

@methane methane methane approved these changes

@serhiy-storchaka serhiy-storchaka serhiy-storchaka approved these changes

@pablogsal pablogsal pablogsal approved these changes

Assignees

@vstinner vstinner

Labels
skip news
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@vstinner @miss-islington @serhiy-storchaka @bedevere-bot @methane @pablogsal @qigangxu @the-knights-who-say-ni
0