Skip to content

gh-148200: fix warning on missing safe memzero() on CYGWIN#148202

Closed
carlo-bramini wants to merge 3 commits into
python:mainfrom
carlo-bramini:fix-cygwin-warning-2
Closed

gh-148200: fix warning on missing safe memzero() on CYGWIN#148202
carlo-bramini wants to merge 3 commits into
python:mainfrom
carlo-bramini:fix-cygwin-warning-2

Conversation

@carlo-bramini

@carlo-bramini carlo-bramini commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

@bedevere-app

bedevere-app Bot commented Apr 7, 2026

Copy link
Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@bedevere-app

bedevere-app Bot commented Apr 7, 2026

Copy link
Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@carlo-bramini

Copy link
Copy Markdown
Contributor Author

I have not added a news entry because this seems to me an extremely trivial change, however let me know if this could be required. Thank you very much.

@picnixz

picnixz commented Apr 9, 2026

Copy link
Copy Markdown
Member

For this, I think we should rather change upstream. I think we changed those values upstream instead (I don't want to diverge from HACL* sources in general).

cc @protz

@protz

protz commented Apr 9, 2026

Copy link
Copy Markdown
Contributor

Thanks I'll replicate the patch upstream

protz added a commit to hacl-star/hacl-star that referenced this pull request Apr 9, 2026
protz added a commit to hacl-star/hacl-star that referenced this pull request Apr 9, 2026
@picnixz

picnixz commented Apr 10, 2026

Copy link
Copy Markdown
Member

@carlo-bramini Could you update the PR so that it pulls the updated upstream instead (update refresh.sh to pull new sources)

@picnixz

picnixz commented Apr 10, 2026

Copy link
Copy Markdown
Member

Sorry, I meant you need to change the refresh.sh script in Modules/_hacl (or something like that) and set the commit hash that contains the upstream fix.

@python-cla-bot

python-cla-bot Bot commented Apr 10, 2026

Copy link
Copy Markdown

All commit authors signed the Contributor License Agreement.

CLA signed

@carlo-bramini

Copy link
Copy Markdown
Contributor Author

The following commit authors need to sign the Contributor License Agreement:

* [30959007+carlo-bramini@users.noreply.github.com](mailto:30959007+carlo-bramini@users.noreply.github.com)

CLA not signed

Excuse me, I had already signed the CLA, what is this?

@carlo-bramini carlo-bramini force-pushed the fix-cygwin-warning-2 branch 2 times, most recently from 1e6383f to f099b51 Compare April 10, 2026 08:49
@picnixz

picnixz commented Apr 10, 2026

Copy link
Copy Markdown
Member

Excuse me, I had already signed the CLA, what is this?

This may happen if you push with a different account / git setting or sometimes web UI

@picnixz

picnixz commented Apr 10, 2026

Copy link
Copy Markdown
Member

FTR, you need to call the refresh.sh script (it seems the upstream commit was not pulled)

@picnixz

picnixz commented Apr 10, 2026

Copy link
Copy Markdown
Member

Oh and you actually need a local HACL* copy for that to work.

@carlo-bramini

carlo-bramini commented Apr 10, 2026

Copy link
Copy Markdown
Contributor Author

@picnixz @protz I have seen that there are several copies of Lib_Memzero0.c into the upstream of HACL.
The patch has been imported only into lib/c/Lib_Memzero0.c, but the refresh.sh script from the sources of Python is importing a file with the same name but from the dist\gcc-compatible directory.
As result, the latest patches are not imported into Python.
How does this thing need to be handled?

@protz

protz commented Apr 10, 2026

Copy link
Copy Markdown
Contributor

Upstream was just fixed via hacl-star/hacl-star#1070 which propagates the hand-written copy into the packaged distributions ("dist").

Sorry for the confusion -- I should've commented here that there were still a few more steps required to fully propagate the fix.

@vstinner

Copy link
Copy Markdown
Member

@protz:

Upstream was just fixed via hacl-star/hacl-star#1070 which propagates the hand-written copy into the packaged distributions ("dist").

I created PR gh-149802 to Modules/_hacl/ for Cygwin.

@vstinner

Copy link
Copy Markdown
Member

I merged #149802 instead. Thanks for your contribution!

@vstinner vstinner closed this May 15, 2026
@carlo-bramini carlo-bramini deleted the fix-cygwin-warning-2 branch May 15, 2026 13:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants