Skip to content

/ cpython

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

Jump to bottom

Fix compiler warnings in MSVC for winreg.c #24441

Merged
merged 1 commit into from Feb 4, 2021
Merged

Fix compiler warnings in MSVC for winreg.c #24441

merged 1 commit into from Feb 4, 2021

Conversation

@Fidget-Spinner
Copy link
Contributor

@Fidget-Spinner Fidget-Spinner commented Feb 4, 2021

During compilation, I get a warning:
1>\Documents\GitHub\cpython\PC\winreg.c(683,21): warning C4018: '<': signed/unsigned mismatch.
Caused by the line assert(len < size); because len is type Py_ssize_t (signed) and size is type DWORD (unsigned).

We can cast len to unsigned safely to silence the warning (which is what the compiler already does implicitly), because there is already a check for assert(len >= 0); above.
@Fidget-Spinner
cast to unsigned
Loading status checks…
6297d71
@Fidget-Spinner Fidget-Spinner requested a review from python/windows-team as a code owner Feb 4, 2021
@Fidget-Spinner
Copy link
Contributor Author

@Fidget-Spinner Fidget-Spinner commented Feb 4, 2021 •
edited

I think this warrants a skip news and skip issue, asserts don't affect release builds AFAIK.

I separated this from #24440 because maybe this needs a backport while that one doesn't.
@zooba
Copy link
Member

@zooba zooba commented Feb 4, 2021

Thanks! I'll do an auto backport and see how far it goes, but it's not essential.
@zooba zooba merged commit 28873a7 into python:master Feb 4, 2021
10 checks passed
10 checks passed
Check for source changes
Details
Check if generated files are up to date
Details
Windows (x86)
Details
Windows (x64)
Details
macOS
Details
Ubuntu
Details
Azure Pipelines PR #20210204.9 succeeded
Details
Travis CI - Pull Request Build Passed
Details
bedevere/issue-number Issue report skipped
bedevere/news "skip news" label found
@miss-islington
Copy link
Contributor

@miss-islington miss-islington commented Feb 4, 2021

Thanks @Fidget-Spinner for the PR, and @zooba for merging it 🌮🎉.. I'm working now to backport this PR to: 3.8, 3.9.
🐍🍒⛏🤖
@miss-islington
Copy link
Contributor

@miss-islington miss-islington commented Feb 4, 2021

Sorry, @Fidget-Spinner and @zooba, I could not cleanly backport this to 3.9 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 28873a70503c9e700fe05c3f9c781dbd6ea19b14 3.9
@miss-islington
Copy link
Contributor

@miss-islington miss-islington commented Feb 4, 2021

Sorry @Fidget-Spinner and @zooba, I had trouble checking out the 3.8 backport branch.
Please backport using cherry_picker on command line.
cherry_picker 28873a70503c9e700fe05c3f9c781dbd6ea19b14 3.8
@zooba
Copy link
Member

@zooba zooba commented Feb 4, 2021

Didn't backport cleanly, so we'll just leave it.
@Fidget-Spinner Fidget-Spinner deleted the Fidget-Spinner:unsigned-signed branch Feb 6, 2021
ncoghlan added a commit to ncoghlan/cpython that referenced this pull request Feb 8, 2021
@Fidget-Spinner @ncoghlan
Fix signed/unsigned comparison to avoid compilation warning (pythonGH…
a5a2035 
…-24441)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@python/windows-team windows-team

Assignees

@zooba zooba

Labels
CLA signed skip issue skip news
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
@Fidget-Spinner @zooba @miss-islington @the-knights-who-say-ni @bedevere-bot