8000 gh-111995: Add getnameinfo extension flag by adder32 · Pull Request #111994 · python/cpython · GitHub
[go: up one dir, main page]

Skip to content

gh-111995: Add getnameinfo extension flag #111994

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

Merged
merged 2 commits into from
Nov 15, 2023

Conversation

adder32
Copy link
Contributor
@adder32 adder32 commented Nov 12, 2023

This PR adds the NI_IDN getnameinfo extension flag.

See: https://man7.org/linux/man-pages/man3/getnameinfo.3.html

The other two flags (NI_IDN_ALLOW_UNASSIGNED, NI_IDN_USE_STD3_ASCII_RULES) are deprecated and I deliberately did not include them in the patch.

Fixes #111995.

@ghost
Copy link
ghost commented Nov 12, 2023

All commit authors signed the Contributor License Agreement.
CLA signed

@bedevere-app
Copy link
bedevere-app bot commented Nov 12, 2023

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.

@adder32 adder32 force-pushed the patch-getnameinfo-flags branch from 78ef4a7 to aa32d0d Compare November 12, 2023 01:08
@bedevere-app
Copy link
bedevere-app bot commented Nov 12, 2023

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.

@adder32 adder32 changed the title Add getnameinfo extension flag gh-111995: Add getnameinfo extension flag Nov 12, 2023
@@ -119,6 +119,7 @@
#define NI_NAMEREQD 0x00000004
#define NI_NUMERICSERV 0x00000008
#define NI_DGRAM 0x00000010
#define NI_IDN 0x00000020
Copy link
Contributor

Choose a reason for hiding this comment

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

This header defines the interface of the getnameinfo implementation in getaddrinfo.c in the same folder, which doesn't support NI_IDN. This implementation is only used on platforms where no suitable getaddrinfo is available in the system.

Therefore adding NI_IDN to this header is not needed.

@adder32 adder32 closed this Nov 13, 2023
@adder32 adder32 reopened this Nov 13, 2023
@adder32
Copy link
Contributor Author
adder32 commented Nov 13, 2023

Let me know if there's anything I can do so that we have NI_IDN on Linux.

@ronaldoussoren
Copy link
Contributor

Let me know if there's anything I can do so that we have NI_IDN on Linux.

Please address my comment about the #define for NI_IDN.

There also needs to be a versionadded in the block below the text mentioning NI_* in Doc/library/socket.rst.

In general I'd ask for a test for this as well, but it looks like other flags aren't tested as well and this is a pretty thin layer for a C API. @gpshead : What's your opinion on new tests for adding a new NI_* constant?

@adder32 adder32 force-pushed the patch-getnameinfo-flags branch from bacdec5 to 0dee9f4 Compare November 15, 2023 00:19
@bedevere-app
Copy link
bedevere-app bot commented Nov 15, 2023

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.

@gpshead
Copy link
Member
gpshead commented Nov 15, 2023

I wouldn't worry about a test for this. it's just trivially exposing a C constant/define. I'll take care of a news entry and merge this.

For use on older versions of CPython the usual applies for any missing constant definitions: Look up the value from C on your target platforms and hard code that as a NI_IDN = value for now in your code.

@gpshead gpshead self-assigned this Nov 15, 2023
@gpshead gpshead enabled auto-merge (squash) November 15, 2023 00:35
@gpshead gpshead merged commit fe9db90 into python:main Nov 15, 2023
@adder32 adder32 deleted the patch-getnameinfo-flags branch November 27, 2023 01:19
aisk pushed a commit to aisk/cpython that referenced this pull request Feb 11, 2024
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
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.

socket module does not include all getnameinfo flags
3 participants
0