Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(255189)

#17561: Add socket.create_server_sock() convenience function

Can't Edit
Can't Publish+Mail
Start Review
Created:
6 years, 4 months ago by g.rodola
Modified:
6 years, 4 months ago
Reviewers:
cf.natali
CC:
loewis, gregory.p.smith, jaraco, nirs, AntoinePitrou, haypo, giampaolo.rodola, josiahcarlson, r.david.murray, asvetlov, Charles-François Natali, berkerpeksag, Martin Panter, pokorny_jan_seznam.cz, jleedev, dazhaoyu_gmail.com, andreas.rosdal_gmail.com, carlos.ralli_gmail.com, pmarks_google.com, csabella
Visibility:
Public.

Patch Set 1 #

Patch Set 2 #

Total comments: 5
Unified diffs Side-by-side diffs Delta from patch set Stats Patch
Lib/socket.py View 1 1 chunk +72 lines, -0 lines 5 comments Download
Lib/test/test_socket.py View 1 1 chunk +57 lines, -0 lines 0 comments Download

Messages

Total messages: 1
Charles-François Natali
6 years, 4 months ago #1
http://bugs.python.org/review/17561/diff/7717/Lib/socket.py
File Lib/socket.py (right):

http://bugs.python.org/review/17561/diff/7717/Lib/socket.py#newcode441
Lib/socket.py:441: listen for both IPv4 and IPv6 connections.
The term "dual stack" is a bit confusing: it just means that the host implements
both IPv4 and IPV6 stacks natively (i.e. you don't have to use tunnels/etc).
Here, we really want to know if it supports IPv4-mapped IPv6 addresses.
See http://tools.ietf.org/html/rfc4213

Also, I'm not convinced we need to expose this yet. We can just provide a best
effort function, someone needing precise control could create the socket
manually (and e.g. use select).

http://bugs.python.org/review/17561/diff/7717/Lib/socket.py#newcode445
Lib/socket.py:445: or not hasattr(_socket, 'IPV6_V6ONLY'):
I think PEP8 discourages line breaks using '\': parenthesis would be better IMO.

http://bugs.python.org/review/17561/diff/7717/Lib/socket.py#newcode460
Lib/socket.py:460: dual_stack=has_dual_stack()):
Same thing here, I think it complicates the signature.
Someone needing precise control can bind the socket by hand, and use select() if
necessary.

http://bugs.python.org/review/17561/diff/7717/Lib/socket.py#newcode463
Lib/socket.py:463: 
The *reuse_addr* argument is not documented in the docstring, and doesn't appear
in any other socket's method signature.
It would probably be good to add a line about it.

Also, I'm not sure it's really wise to enable SO_REUSEADDR by default: it's
practical, but it's not the default on BSD sockets.

Furthermore, having a default value different between Unix and Windows is bad
for portability.

Finally, "explicit is better than implicit" ;-)

http://bugs.python.org/review/17561/diff/7717/Lib/socket.py#newcode477
Lib/socket.py:477: AF_INET6_ = getattr(_socket, "AF_INET6", 0)
It might be possible to use socket.has_ipv6.
Sign in to reply to this message.

RSS Feeds Recent Issues | This issue
This is Rietveld 894c83f36cb7+