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 in socketmodule.c#131821

Merged
zooba merged 3 commits intopython:mainfrom
chris-eibl:fix_clangcl_socketmodule
Jun 9, 2025
Merged

GH-131296: fix clang-cl warning on Windows in socketmodule.c#131821
zooba merged 3 commits intopython:mainfrom
chris-eibl:fix_clangcl_socketmodule

Conversation

@chris-eibl
Copy link
Member

@chris-eiblchris-eibl commentedMar 28, 2025
edited by bedevere-appbot
Loading

#ifdef MS_WINDOWS
if (optname == SIO_TCP_SET_ACK_FREQUENCY) {
int dummy;
DWORD dummy;
Copy link
MemberAuthor

Choose a reason for hiding this comment

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

fix warning : incompatible pointer types passing 'int *' to parameter of type 'LPDWORD' (aka 'unsigned long *') [-Wincompatible-pointer-types]

PyThread_acquire_lock(netdb_lock, 1);
#endif
SUPPRESS_DEPRECATED_CALL
_Py_COMP_DIAG_PUSH
Copy link
MemberAuthor

Choose a reason for hiding this comment

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

The rest of the PR cares about deprecation warnings like

..\Modules\socketmodule.c(6070,9): warning : 'gethostbyname' is deprecated:Use getaddrinfo() or GetAddrInfoW() instead or define _WINSOCK_DEPRECATED_NO_WARNINGSto disable deprecated API warnings [-Wdeprecated-declarations]

https://github.com/python/cpython/actions/runs/14044831663/job/39366560293?pr=131690#step:4:183

using the already existing infrastructure_Py_COMP_DIAG_PUSH et al. frompyport.h, because clang-cl unfortunately does not (yet) respect

# defineSUPPRESS_DEPRECATED_CALL __pragma(warning(suppress: 4996))

Copy link
Member

Choose a reason for hiding this comment

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

Is there another way to spell the pragma that will work? The idea ofpyport.h is to deal with compiler differences as much as possible, so it's the place to handle it if the original macro can be made to work.

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

clang-cl does not understandsuppress, hence I need to push and pop and thus used the existing infrastructure frompyport.h (warning(disable: 4996))

#elif defined(_MSC_VER)
#define_Py_COMP_DIAG_PUSH__pragma(warning(push))
#define_Py_COMP_DIAG_IGNORE_DEPR_DECLS__pragma(warning(disable:4996))
#define_Py_COMP_DIAG_POP__pragma(warning(pop))

Likewise, AFAIK, there is nosupress in gcc / "regular" clang, so we need to always push / pop there.

zooba reacted with thumbs up emoji
@chris-eibl
Copy link
MemberAuthor

I am pretty sure that failing ofUbuntu (free-threading) / build and test (ubuntu-24.04) (pull_request) is unrelated to the PR.

@python-cla-bot
Copy link

All commit authors signed the Contributor License Agreement.

CLA signed

@chris-eibl
Copy link
MemberAuthor

@zooba May I ask you for a review?

Comment on lines 6173 to 6174
_Py_COMP_DIAG_POP
#endif /* HAVE_GETHOSTBYNAME_R */
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Two space indents of preprocessor directives might be uncommon in CPython except when a line starts with#.

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

I indented everywhere by two spaces to easier spot the macros, but I will happily indent by four spaces if this is preferred.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd prefer to avoid thinking of the intention when reading, so +1 for four spaces.

chris-eibl and zooba reacted with thumbs up emoji
Copy link
MemberAuthor

Choose a reason for hiding this comment

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

Done. I had a look at the other occurrences in the code base: either no indentation or like the code they guard.

I switched to the latter.

neonene reacted with heart emoji
@zooba
Copy link
Member

LGTM. I'll merge after CI is done unless something else comes up (or if someone else sees this and can merge, go ahead)

chris-eibl reacted with heart emoji

@zoobazooba merged commitcc8e6d2 intopython:mainJun 9, 2025
38 checks passed
@chris-eiblchris-eibl deleted the fix_clangcl_socketmodule branchJune 9, 2025 16:48
Pranjal095 pushed a commit to Pranjal095/cpython that referenced this pull requestJul 12, 2025
taegyunkim pushed a commit to taegyunkim/cpython that referenced this pull requestAug 4, 2025
Agent-Hellboy pushed a commit to Agent-Hellboy/cpython that referenced this pull requestAug 19, 2025
hunterhogan added a commit to hunterhogan/cpython that referenced this pull requestMar 14, 2026
@chris-eiblchris-eibl added the needs backport to 3.14bugs and security fixes labelMar 23, 2026
@miss-islington-app
Copy link

Thanks@chris-eibl for the PR, and@zooba for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull requestMar 23, 2026
…GH-131821)(cherry picked from commitcc8e6d2)Co-authored-by: Chris Eibl <138194463+chris-eibl@users.noreply.github.com>
@bedevere-app
Copy link

GH-146339 is a backport of this pull request to the3.14 branch.

@bedevere-appbedevere-appbot removed the needs backport to 3.14bugs and security fixes labelMar 23, 2026
ambv pushed a commit that referenced this pull requestMar 23, 2026
…1821) (GH-146339)(cherry picked from commitcc8e6d2)Co-authored-by: Chris Eibl <138194463+chris-eibl@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@zoobazoobazooba left review comments

+1 more reviewer

@neoneneneoneneneonene left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@chris-eibl@zooba@neonene@picnixz

[8]ページ先頭

©2009-2026 Movatter.jp