-
-
Notifications
You must be signed in to change notification settings - Fork 31.4k
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
gh-114917: add support for AI_NUMERICSERV in getaddrinfo emulation #114918
Conversation
maxbachmann
commented
Feb 2, 2024
•
edited by bedevere-app
bot
Loading
edited by bedevere-app
bot
- Issue: add support for AI_NUMERICSERV in getaddrinfo emulation #114917
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you write a test for this feature?
I am not sure which currently tested target even runs this code. This is only included conditionally via: #if !defined(HAVE_GETNAMEINFO)
#define getnameinfo fake_getnameinfo
#include "getnameinfo.c"
#endif // HAVE_GETNAMEINFO |
I added a test. However this only tests the fallback if we actually have a target using it that is under testing. |
This comment was marked as off-topic.
This comment was marked as off-topic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I just have a minor coding style request.
@vstinner is there still something stopping this from getting merged? |
I don't know Modules/getaddrinfo.c. Which platforms are using it? It looks like dead code to me. It starts with a big My
but |
I tried to using this Linux with the patch: diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c
index 916ad35623e..c49d0d53042 100644
--- a/Modules/socketmodule.c
+++ b/Modules/socketmodule.c
@@ -448,6 +448,8 @@ remove_unusable_flags(PyObject *m)
#endif
+#undef HAVE_GETADDRINFO
+
/* I know this is a bad practice, but it is the easiest... */
#if !defined(HAVE_GETADDRINFO)
/* avoid clashes with the C library definition of the symbol. */ On Linux, I get many build errors:
How did you test your change? Are using using macOS? |
It's only used when
I assume you didn't undef this for enough code. The include of
We are using it in our playstation port of CPython |
Oh ok. A few lines to support PlayStation sounds reasonable, I merged your PR. Thanks. |