Skip to content

gh-96853: Restore test coverage for Py_Initialize(Ex) #98212

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

Conversation

ncoghlan
Copy link
Contributor

@ncoghlan ncoghlan commented Oct 12, 2022

  • As most of test_embed now uses Py_InitializeFromConfig, add a specific test case to cover Py_Initialize(Ex)
  • Rename _testembed init helper to clarify the API used
  • Add a PyConfig_Clear call in Py_InitializeEx to make the code more obviously correct (it already didn't leak as none of the dynamically allocated config fields were being populated, but it's clearer if the wrappers follow the documented API usage guidelines)

* As most of `test_embed` now uses `Py_InitializeFromConfig`, add
  a specific test case to cover `Py_Initialize(Ex)`
* Rename `_testembed` init helper to clarify the API used
* Add a `PyConfig_Clear` call in `Py_InitializeEx` to make
  the code more obviously correct (it already didn't leak as
  none of the dynamically allocated config fields were being
  populated, but it's clearer if the wrappers follow the
  documented API usage guidelines)
@ncoghlan ncoghlan requested a review from vstinner October 12, 2022 06:10
@ncoghlan ncoghlan changed the title Gh 96853 missing pyconfig clear in py initializeex gh-96853: Restore test coverage for Py_Initialize(Ex) Oct 12, 2022
@ncoghlan ncoghlan added 3.11 only security fixes 3.12 only security fixes needs backport to 3.11 only security fixes labels Oct 16, 2022
@ncoghlan
Copy link
Contributor Author

@vstinner Did you want to take another look at this minor Py_InitializeEx fix? If there aren't any further comments I'll go ahead and merge it this coming Wed (26 Oct)

@ncoghlan ncoghlan merged commit 05e4886 into python:main Oct 30, 2022
@miss-islington
Copy link
Contributor

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

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 30, 2022
…-98212)

* As most of `test_embed` now uses `Py_InitializeFromConfig`, add
  a specific test case to cover `Py_Initialize` (and `Py_InitializeEx`)
* Rename `_testembed` init helper to clarify the API used
* Add a `PyConfig_Clear` call in `Py_InitializeEx` to make
  the code more obviously correct (it already didn't leak as
  none of the dynamically allocated config fields were being
  populated, but it's clearer if the wrappers follow the
  documented API usage guidelines)
(cherry picked from commit 05e4886)

Co-authored-by: Nick Coghlan <ncoghlan@gmail.com>
@ZeroIntensity ZeroIntensity removed 3.11 only security fixes 3.12 only security fixes needs backport to 3.11 only security fixes labels Feb 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants