Add sendmsg(), recvmsg(), recvmsg_into() methods to Python sockets

From http://bugs.python.org/issue6560

Based on Heiko Wundram's patch from
http://bugs.python.org/issue1194378

[Antoine Pitrou, 2010-06-10 19:05:20]

Here is a couple of comments that came to my mind while reading the patch.
Overall the patch looks good, although I'm not an expert in the field.
I also tested it works fine on recent x86-64 Linux system.

http://codereview.appspot.com/1487041/diff/1/2
File Doc/library/socket.rst (right):

http://codereview.appspot.com/1487041/diff/1/2#newcode450
Doc/library/socket.rst:450: .. function:: CMSG_LEN(length)
The documentation for both CMSG_LEN and CMSG_SPACE is quite cryptic IMO. Would
it be possible to state what they are useful for?

http://codereview.appspot.com/1487041/diff/1/5
File Lib/test/test_socket.py (right):

http://codereview.appspot.com/1487041/diff/1/5#newcode389
Lib/test/test_socket.py:389: class TCPTestBase(InetTestBase):
Is such a forest of subclasses and mixins really needed? For example, TCP will
always function in connected mode and UDP in connectionless mode.

http://codereview.appspot.com/1487041/diff/1/5#newcode1297
Lib/test/test_socket.py:1297: self.assertIsNone(cm.exception.errno)
This line should be indented to the left, otherwise it doesn't get executed (the
previous line raises an error and leaves the "with" scope).

[baikie, 2010-06-10 21:50:48]

Thanks for wading through the patch - especially all that test code.  I've been
testing on 32-bit only, so it's good to know that it works on 64-bit Linux.

http://codereview.appspot.com/1487041/diff/1/2
File Doc/library/socket.rst (right):

http://codereview.appspot.com/1487041/diff/1/2#newcode450
Doc/library/socket.rst:450: .. function:: CMSG_LEN(length)
On 2010/06/10 19:05:20, Antoine Pitrou wrote:
> The documentation for both CMSG_LEN and CMSG_SPACE is quite cryptic IMO. Would
> it be possible to state what they are useful for?

Yes, I suppose it doesn't make much sense if you haven't read the recvmsg()
docs.  I could perhaps add something about the need to restrict the amount of
ancillary data received.

Using CMSG_LEN() to size the ancillary buffer is actually not officially
sanctioned, but CMSG_SPACE() might leave too much space and allow an unwanted
extra file descriptor to be received, for example (I haven't actually seen this,
but I presume it can happen on a 64-bit system).

http://codereview.appspot.com/1487041/diff/1/5
File Lib/test/test_socket.py (right):

http://codereview.appspot.com/1487041/diff/1/5#newcode389
Lib/test/test_socket.py:389: class TCPTestBase(InetTestBase):
On 2010/06/10 19:05:20, Antoine Pitrou wrote:
> Is such a forest of subclasses and mixins really needed? For example, TCP will
> always function in connected mode and UDP in connectionless mode.

Perhaps it could be tidied up, but I modelled them on the existing classes such
as SocketTCPTest and SocketConnectedTest with a view to replacing those
(SocketTCPTest doesn't connect the socket).  I just didn't want to make the
patch actually delete them yet.

You can actually connect a UDP socket - it will then report the errors you got
from ICMP or the local network stack, etc.  I don't know, maybe I should have
tested sendmsg with that as well :)

http://codereview.appspot.com/1487041/diff/1/5#newcode1297
Lib/test/test_socket.py:1297: self.assertIsNone(cm.exception.errno)
On 2010/06/10 19:05:20, Antoine Pitrou wrote:
> This line should be indented to the left, otherwise it doesn't get executed
(the
> previous line raises an error and leaves the "with" scope).

Well spotted, thanks.

[Antoine Pitrou, 2010-06-12 17:18:21]

I forgot one small thing.

http://codereview.appspot.com/1487041/diff/1/5
File Lib/test/test_socket.py (right):

http://codereview.appspot.com/1487041/diff/1/5#newcode1041
Lib/test/test_socket.py:1041: class TestError(Exception):
You don't need this. Just use self.fail().

[baikie, 2010-06-13 19:49:48]

Here is a revised version of the patch.  I've done some minor cleanup in the
tests, fixed the cm.exception.errno line, and clarified the CMSG_*() docs and
made them link to recvmsg().

The TestError thing in checkFlags is meant to indicate an incorrectly-written
test rather than a failing one.  I can't remember why I made a new exception for
it though, so I've just made it raise Exception (so it won't get caught by
mistake).

I made a separate patch to replace SocketTCPTest, etc. with the base/mixin
classes from the sendmsg patch, which I'll post back at the Tracker.  With that
done, I didn't see an obvious way to reduce the number of classes, except
perhaps making the stream servers listen unconditionally, or merging
ThreadedSocketTestMixin into the base (but then all the tests would need
threading available).

[baikie, 2010-06-13 19:50:18]

Revised version of patch

[baikie, 2010-06-13 19:56:52]

Argh, I shouldn't have used quilt.  Let's try that again.

[baikie, 2010-12-06 19:52:34]

Updated to use new BEGIN/END_SELECT_LOOP macros, fixed interrupted-timeout bug