Sockets support for CAN_BCM

[Charles-François Natali]

Here's a review.

The test is fine (except for the missing padding and size/alignment issue, see
below comments). If you can add one or two more tests, it'd be great, otherwise
it's probably enough.

You must update the documentation, but it's not much work, see for example the
commit adding AF_CAN support:

http://hg.python.org/cpython/rev/e767318baccd/

http://bugs.python.org/review/15359/diff/7130/Lib/test/test_socket.py
File Lib/test/test_socket.py (right):

http://bugs.python.org/review/15359/diff/7130/Lib/test/test_socket.py#newcode...
Lib/test/test_socket.py:1393: 
You must skip this test is CAN sockets are not supported:
"""
@unittest.skipUnless(HAVE_SOCKET_CAN, 'SocketCan required for this test.')
@unittest.skipUnless(thread, 'Threading required for this test.')
"""

http://bugs.python.org/review/15359/diff/7130/Lib/test/test_socket.py#newcode...
Lib/test/test_socket.py:1431: """Must use native not standard types for packing
You should move the comment and the struct format in the class definition before
the methods.

http://bugs.python.org/review/15359/diff/7130/Lib/test/test_socket.py#newcode...
Lib/test/test_socket.py:1443: bcm_cmd_msg_fmt = "@IIIllllII"
I got a deadlock during the test, because sending on the BCM socket returned
EINVAL.
The problem is that the struct definition uses the zero-length array trick, but
on my box sizeof(struct bcm_msg_head) returns 40 instead of 36, because
can_frame must be 8 bytes aligned (data field).

So in short, I think the format should be:
"=IIIllllII4x"

'=' because you want native endianess, but standard size and alignment.

http://bugs.python.org/review/15359/diff/7130/Lib/test/test_socket.py#newcode...
Lib/test/test_socket.py:4887: """
What's this?

http://bugs.python.org/review/15359/diff/7130/Modules/socketmodule.c
File Modules/socketmodule.c (right):

http://bugs.python.org/review/15359/diff/7130/Modules/socketmodule.c#newcode1638
Modules/socketmodule.c:1638: case CAN_BCM:
This switch block is exactly the same as when the socket protocol is CAN_RAW
(except for the s->sock_type = SOCK_DGRAM part should not be necessary, since
the socket type should be set): you should probably just use a fallthrough:
"""
    case AF_CAN:
        switch (s->sock_proto) {
        case CAN_RAW:
        /* fallthrough */
        case CAN_BCM:
        <original CAN_RAW block here>
"""

http://bugs.python.org/review/15359/diff/7130/configure
File configure (right):

http://bugs.python.org/review/15359/diff/7130/configure#newcode7215
configure:7215: for ac_header in linux/can.h linux/can/raw.h linux/can/bcm.h
Just a remark: we usually don't post configure diff: it's generated from
configure.ac, and can be quite large (e.g. if you're not using the same autoconf
version as the one used to build the current configure).

[Brian]

http://bugs.python.org/review/15359/diff/7130/Lib/test/test_socket.py
File Lib/test/test_socket.py (right):

http://bugs.python.org/review/15359/diff/7130/Lib/test/test_socket.py#newcode...
Lib/test/test_socket.py:1393: 
On 2013/01/16 22:28:42, Charles-François Natali wrote:
> You must skip this test is CAN sockets are not supported:
> """
> @unittest.skipUnless(HAVE_SOCKET_CAN, 'SocketCan required for this test.')
> @unittest.skipUnless(thread, 'Threading required for this test.')
> """
I've removed this class. It had inherited from CANTest so functionally this
already works. Is it necessary just for code readability? I notice all the
skipUnless threading could be refactored into two places too.

http://bugs.python.org/review/15359/diff/7130/Lib/test/test_socket.py#newcode...
Lib/test/test_socket.py:1431: """Must use native not standard types for packing
On 2013/01/16 22:28:42, Charles-François Natali wrote:
> You should move the comment and the struct format in the class definition
before
> the methods.

Done.

http://bugs.python.org/review/15359/diff/7130/Lib/test/test_socket.py#newcode...
Lib/test/test_socket.py:1443: bcm_cmd_msg_fmt = "@IIIllllII"
On 2013/01/16 22:28:42, Charles-François Natali wrote:
> I got a deadlock during the test, because sending on the BCM socket returned
> EINVAL.
> The problem is that the struct definition uses the zero-length array trick,
but
> on my box sizeof(struct bcm_msg_head) returns 40 instead of 36, because
> can_frame must be 8 bytes aligned (data field).
> 
> So in short, I think the format should be:
> "=IIIllllII4x"
> 
> '=' because you want native endianess, but standard size and alignment.

Hmm using a format of "=IIIllllII4x" causes the test to fail on my box. We will
need to rethink this further.

http://bugs.python.org/review/15359/diff/7130/Lib/test/test_socket.py#newcode...
Lib/test/test_socket.py:4887: """
On 2013/01/16 22:28:42, Charles-François Natali wrote:
> What's this?

My mistake, I just commented out the other tests while developing.