Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

gh-131296: Fix clang-cl warning on Windows inposixmodule.c#133142

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
zooba merged 2 commits intopython:mainfromWulian233:posix-warn
May 5, 2025

Conversation

Wulian233
Copy link
Contributor

@Wulian233Wulian233 commentedApr 29, 2025•
edited by bedevere-appbot
Loading

Only 1 warning inposixmodule.c

..\Modules\posixmodule.c(4193,17): warning : result of comparison of constant 4611686018427387903 with expression of type 'DWORD' (aka 'unsigned long') is always true [-Wtautological-constant-out-of-range-compare]

Please add skip news label :)

Copy link

@auvipyauvipy left a comment

Choose a reason for hiding this comment

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

looks good. do we also need to update relevant test for this change?

@Wulian233
Copy link
ContributorAuthor

Generally not. We only fix some compilation warnings and did not make obviously change. can refer to#131296

@@ -4190,7 +4190,7 @@ posix_getcwd(int use_bytes)
terminating \0. If the buffer is too small, len includes
the space needed for the terminator. */
if (len >= Py_ARRAY_LENGTH(wbuf)) {
if (len <= PY_SSIZE_T_MAX / sizeof(wchar_t)) {
if ((Py_ssize_t)len <= PY_SSIZE_T_MAX / sizeof(wchar_t)) {
Copy link
Member

Choose a reason for hiding this comment

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

This indeed fixes the warning. IMHO, that's the most pragmatic fix. We could be fancy and do some#if onsizeof(Py_ssize_t), etc, but I don't think it is worth the hassle.

Wulian233 reacted with thumbs up emoji
Copy link
Member

Choose a reason for hiding this comment

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

sizeof doesn't work in preprocessor checks anyway.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I meant something like we do withSIZEOF_SIZE_T, but did not want to elaborate about it in great detail, because IMHO it isn't worth the churn?

@Wulian233
Copy link
ContributorAuthor

@zooba Could you review and merge it, thank you!

@zooba
Copy link
Member

It's a bit of an annoying warning, because they'd be justified in still warning even with the cast (since itstill can't ever be larger than that value for a given architecture), but there's nothing more we can do about it. If this cast ever stops suppressing the warning, then we probably just have to disable the warning.

@zoobazooba merged commitb6c2ef0 intopython:mainMay 5, 2025
45 checks passed
@Wulian233Wulian233 deleted the posix-warn branchMay 5, 2025 22:20
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@zoobazoobazooba left review comments

@auvipyauvipyauvipy left review comments

@chris-eiblchris-eiblchris-eibl approved these changes

@DangNhutNguyenDangNhutNguyenDangNhutNguyen approved these changes

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

Successfully merging this pull request may close these issues.

6 participants
@Wulian233@zooba@auvipy@chris-eibl@DangNhutNguyen@aisk
[8]ページ先頭
©2009-2025 Movatter.jp