bpo-31425: Expose AF_QIPCRTR in socket module #3706
Conversation
|
Hello, and thanks for your contribution! I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA). Unfortunately we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA (this might be simply due to a missing "GitHub Name" entry in your b.p.o account settings). This is necessary for legal reasons before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue. Thanks again to your contribution and we look forward to looking at it! |
|
The CLA is signed, but unfortunately I did not have the "Github name" filled in on my account. This is corrected now. |
andersson
force-pushed
the
bpo-31425
branch
2 times, most recently
from
e09b830 to
860777d
Compare
| #if defined(AF_QIPCRTR) | ||
| case AF_QIPCRTR: | ||
| { | ||
| *len_ret = sizeof (struct sockaddr_qrtr); |
There was a problem hiding this comment.
please remove the extra space after sizeof
There was a problem hiding this comment.
For the record, I'm ignoring this to follow the coding style in this function.
| @@ -54,6 +54,15 @@ typedef int socklen_t; | |||
| # undef AF_NETLINK | |||
| #endif | |||
|
|
|||
| #ifdef HAVE_LINUX_QRTR_H | |||
| # ifdef HAVE_ASM_TYPES_H | |||
| # include <asm/types.h> | |||
There was a problem hiding this comment.
You must add the new addr type to the large sock_addr_t union.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
andersson
force-pushed
the
bpo-31425
branch
2 times, most recently
from
e58b18e to
12e9198
Compare
The AF_QIPCRTR address family was introduced in Linux v4.7, expose this in the Python socket module to allow applications to operate on sockets of this type. Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
There was a problem hiding this comment.
PR looks fine to me
@vstinner @serhiy-storchaka Can you have a look, please? We don't have a dedicated expert for the socket module. You have touched some code recently, too.
|
@andersson I'm sorry that your PR didn't make it into Python 3.7. Please rebase your PR on master and address @serhiy-storchaka's comments. |
|
@andersson Any chance you would be able to respond to @tiran and @serhiy-storchaka 's requests? Seems like this is close to being able to be merged. Thanks! |
# Conflicts: # Doc/library/socket.rst # Lib/test/test_socket.py
|
Due to this being nearly ready to go but untouched for over 6 months, I've gone ahead and addressed @serhiy-storchaka's latest review comments, and merged the latest master branch in. @serhiy-storchaka, @tiran, please consider reviewing again. |
|
Thank you @taleinat! All my comments have been addressed. |
|
Thanks for picking this up @taleinat and thanks for merging it @serhiy-storchaka! |
| family is represented as a ``(node, port)`` tuple where the *node* and *port* | ||
| are non-negative integers. | ||
|
|
||
| .. versionadded:: 3.7 |
There was a problem hiding this comment.
This should be 3.8, not 3.7.
The AF_QIPCRTR address family was introduced in Linux v4.7, expose this
in the Python socket module to allow applications to operate on sockets
of this type.
Signed-off-by: Bjorn Andersson bjorn.andersson@linaro.org
https://bugs.python.org/issue31425