X Tutup
The Wayback Machine - https://web.archive.org/web/20241214214353/https://github.com/python/cpython/pull/1348
Skip to content
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

bpo-27377: Add socket.fdtype() #1348

Closed
wants to merge 1 commit into from

Conversation

nascheme
Copy link
Member

@nascheme nascheme commented Apr 28, 2017

This is a recreation of pull request #1333 as there seems no way to move the source branch into my own repo. I have removed the redundant definitions as pointed out by @tiran. I could maybe be convinced that socket.socket() get this functionality as suggested.

Comment from @tiran copied here::

I'm -1 on fromfd2. We shouldn't introduce yet another function and bloat the API even further. Instead
let's fix the underlying bug and make socket.socket(fileno=fd) detect the socket values, see
https://bugs.python.org/issue28134


Availability: Unix.

.. versionadded:: 3.6
Copy link
Contributor

@DimitrisJim DimitrisJim Apr 28, 2017

Choose a reason for hiding this comment

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

Shouldn't the versionadded (and similarly for fdtype) be 3.7?

@tiran
Copy link
Member

tiran commented Apr 28, 2017

#1349 implements a fix that does not require a new API and fixes all existing usage of socket(fileno) automatically.

Get socket information from a file descriptor.  The return value is a
3-tuple with the following structure: (family, type, proto).  Not
supported on all platforms.
@nascheme nascheme changed the title bpo-27377: Add socket.fromfd2() and socket.fdtype() bpo-27377: Add socket.fdtype() Apr 28, 2017
@nascheme
Copy link
Member Author

Changed title, I think with the socket.socket change proposed by @tiran, we don't need fromf2(). I still think socket.fdtype() is potentially useful.

As suggested by @DimitrisJim, I have fixed the version number to be 3.7.

The HAVE_FDTYPE define may need some tweaking yet. Based on testing I did some months ago, platforms define SO_TYPE and SO_PROTOCOL but the fdtype() implementation doesn't work on them.

Having this function in socket would allow the unit tests for #1349 to check for it and only run the tests if fdtype() is supported on the platform.

}

l = sizeof(sa);
if (getsockname(fd, &sa, &l) < 0) {
Copy link
Member

Choose a reason for hiding this comment

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

Your code doesn't work on Windows for IPv6 addresses. You have to make the buffer large enough to fit all possible address types in it.

sock_addr_t addrbuf;
socklen_t addrlen = sizeof(sock_addr_t);

memset(&addrbuf, 0, addrlen);
if (getsockname(fd, SAS2SA(&addrbuf), &addrlen) == 0) {
    family = SAS2SA(&addrbuf)->sa_family;
} else {
    return set_error();
}

@brettcannon brettcannon added the type-feature A feature request or enhancement label May 1, 2017
@nascheme
Copy link
Member Author

nascheme commented Feb 7, 2019

Closing because Christian Heimes wants to fix a different way (which sounds fine to me).

@nascheme nascheme closed this Feb 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting review type-feature A feature request or enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants
X Tutup