8000 bpo-37732 : Fix unintialized pointer warning in Objects/obmalloc.c by hansrajdas · Pull Request #15309 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

bpo-37732 : Fix unintialized pointer warning in Objects/obmalloc.c #15309

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
Closed
wants to merge 1 commit into from
Closed

bpo-37732 : Fix unintialized pointer warning in Objects/obmalloc.c #15309

wants to merge 1 commit into from

Conversation

hansrajdas
Copy link
Contributor
@hansrajdas hansrajdas commented Aug 15, 2019 •
edited by bedevere-bot
Loading
  • Initialize ptr with NULL pointer while declaring

warning-2019-08-15 01-15-06

https://bugs.python.org/issue37732

@hansrajdas
Fix unintialized pointer warning in Objects/obmalloc.c
2da5f25 
* Initialize ptr with NULL pointer while declaring
nanjekyejoannah
nanjekyejoannah reviewed Aug 16, 2019
View reviewed changes
Copy link
Contributor
@nanjekyejoannah nanjekyejoannah left a comment

Choose a reason for hiding this comment

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

I had already opened a bpo issue here for this : https://bugs.python.org/issue37732 for someone to investigate. Please link to it in this PR title.
i.e

bpo-37732 : Fix unintialized pointer warning in Objects/obmalloc.c

@hansrajdas hansrajdas changed the title Fix unintialized pointer warning in Objects/obmalloc.c bpo-37732 : Fix unintialized pointer warning in Objects/obmalloc.c Aug 16, 2019
@hansrajdas
Copy link
Contributor Author
hansrajdas commented Aug 16, 2019 •
edited by bedevere-bot
Loading

I had already opened a bpo issue here for this : https://bugs.python.org/issue37732 for someone to investigate. Please link to it in this PR title.
i.e

bpo-37732 : Fix unintialized pointer warning in Objects/obmalloc.c

Thanks @nanjekyejoannah, I will link this PR with opened bpo issue. And also move this PR from draft to open state.

@hansrajdas hansrajdas marked this pull request as ready for review August 16, 2019 20:28
@methane
Copy link
Member
methane commented Aug 17, 2019

Why no warning in _PyObject_Calloc?

@hansrajdas
Copy link
Contributor Author

Why no warning in _PyObject_Calloc?

This is because in _PyObject_Calloc memory is initialized using memset but in case of _PyObject_Malloc memory is just allocated and returned(no initialization is done). If we remove memset statement from _PyObject_Calloc we will get the same pointer uninitialized warning for _PyObject_Calloc also. I have verified this.

This is another PR #15293 where Victor suggested that _PyObject_Malloc was intentionally designed to return uninitialized memory so we can't use memset here to fix this warning so I have just initialized ptr with NULL pointer to fix this warning. Please guide me if we can do anythig better to fix this warning.

@qigangxu
Copy link
Contributor
qigangxu commented Aug 18, 2019 •
edited by bedevere-bot
Loading

It seems that warning was introduced by bpo-37543: optimize pymalloc (#14674

Why does this warning appear after using 'LIKELY'?

@qigangxu
Copy link
Contributor

And maybe we can resue _PyObject_Malloc in _PyObject_Calloc.

@vstinner vstinner mentioned this pull request Aug 19, 2019
Merged
@vstinner
Copy link
Member

I dislike this change since it adds an useless initialization to NULL, ptr is already always initialized: GCC is just wrong. I wrote PR #15333 to fix the warning without adding an useless initialization.

_PyObject_Malloc() function is performance critical so I would prefer to not make it slower ;-)

@vstinner
Copy link
Member

Close: I merged my PR #15333 instead.

@vstinner vstinner closed this Aug 20, 2019
@vstinner
Copy link
Member

@hansrajdas: but thanks for your contribution anyway ;-)

@vstinner vstinner reopened this Aug 20, 2019
@vstinner vstinner closed this Aug 20, 2019
@hansrajdas
Copy link
Contributor Author

Thanks @vstinner for your guidance, It was good learning for me following your PR #15333 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nanjekyejoannah nanjekyejoannah nanjekyejoannah left review comments

Assignees
No one assigned
Labels
awaiting review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@hansrajdas @methane @qigangxu @vstinner @nanjekyejoannah @the-knights-who-say-ni @bedevere-bot
0