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

SocketCan support #54350

Closed
slanden mannequin opened this issue Oct 19, 2010 · 42 comments
Closed

SocketCan support #54350

slanden mannequin opened this issue Oct 19, 2010 · 42 comments
Labels
topic-IO type-feature A feature request or enhancement

Comments

@slanden
Copy link
Mannequin

slanden mannequin commented Oct 19, 2010

BPO 10141
Nosy @vsajip, @amauryfa, @orsenthil, @pitrou, @vstinner, @giampaolo, @bitdancer, @vadmium, @xrmx
PRs
  • bpo-42653: Add constants for isotp.h which is included in Linux >= 5.10 #23794
  • Files
  • socketcan.dpatch: SocketCan patch for 2.6.5
  • socketcan.patch: SocketCan patch for 3.3
  • socketcan_v2.patch: SocketCan patch v2 for 3.3
  • socketcan_v5.patch
  • af_can_undefined.diff
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2015-12-04.09:13:48.302>
    created_at = <Date 2010-10-19.00:49:25.052>
    labels = ['type-feature', 'expert-IO']
    title = 'SocketCan support'
    updated_at = <Date 2020-12-16.08:54:40.280>
    user = 'https://bugs.python.org/slanden'

    bugs.python.org fields:

    activity = <Date 2020-12-16.08:54:40.280>
    actor = 'python-dev'
    assignee = 'none'
    closed = True
    closed_date = <Date 2015-12-04.09:13:48.302>
    closer = 'vinay.sajip'
    components = ['IO']
    creation = <Date 2010-10-19.00:49:25.052>
    creator = 'slanden'
    dependencies = []
    files = ['19277', '22702', '22824', '23234', '23337']
    hgrepos = []
    issue_num = 10141
    keywords = ['patch', 'needs review']
    message_count = 42.0
    messages = ['119096', '119120', '119122', '119123', '121125', '139763', '139786', '140713', '141058', '141534', '144416', '144434', '144476', '144777', '144959', '144960', '144961', '145028', '145029', '145032', '145070', '145081', '145083', '145088', '145093', '145100', '145101', '145103', '145107', '145109', '145143', '145165', '214213', '214262', '214308', '214328', '214343', '214349', '214391', '255792', '297725', '297795']
    nosy_count = 15.0
    nosy_names = ['vinay.sajip', 'amaury.forgeotdarc', 'orsenthil', 'pitrou', 'vstinner', 'giampaolo.rodola', 'r.david.murray', 'neologix', 'slanden', 'rosslagerwall', 'python-dev', 'ogait87', 'mluis', 'martin.panter', 'Riccardo Magliocchetti']
    pr_nums = ['23794']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue10141'
    versions = ['Python 3.3']

    @slanden
    Copy link
    Mannequin Author

    slanden mannequin commented Oct 19, 2010

    @slanden slanden mannequin added topic-IO type-feature A feature request or enhancement labels Oct 19, 2010
    @orsenthil
    Copy link
    Member

    Looks like an interesting feature request. What are the packages or external libraries which may rely on this and how do they deal with SocketCan support at moment for their requirements? (That thread mentions several, but some details may help further).

    BTW, this can go in Python 3.2 if it gets in before Nov 2nd week. It wont be going into Python 2.x series.

    @slanden
    Copy link
    Mannequin Author

    slanden mannequin commented Oct 19, 2010

    forgot to include this: http://en.wikipedia.org/wiki/Socketcan

    SocketCan is the (currently) Linux-specific Berkley-socket CAN implementation created by Volkswagen. Alot of previous CAN implementations use a rather inferior (explained in can.txt in the Linux sources) character device approach.

    So the use case for programs that use SocketCan is raw sockets in C, as it is a rather specialty form of interface. But considering that it fits fairly well into an existing interface, it seems appropriate to add to python.

    I clicked 2.7 because that was what the patch was written for.

    Our personal use case is for an IVI dash, to access vehicle devices that output familiar metrics like speed, tach, fuel levels, seat belts, etc over CAN.

    @slanden
    Copy link
    Mannequin Author

    slanden mannequin commented Oct 19, 2010

    • PF_CAN family of sockets

    @amauryfa
    Copy link
    Member

    The patch looks good at first glance, but is there a way to test the feature?

    @ogait87
    Copy link
    Mannequin

    ogait87 mannequin commented Jul 4, 2011

    There is this "simple" program that can be easily adapted to test the interface. http://old.nabble.com/Socketcan-with-Python-td29286297.html#a29286297

    @neologix
    Copy link
    Mannequin

    neologix mannequin commented Jul 4, 2011

    The patch looks good to me.
    A couple remarks:

    • could you please post it as a Mercurial diff? (it makes it easier to
      review and apply)

    @@ -1151,6 +1151,25 @@ makesockaddr(int sockfd, struct sockaddr
    }

    + return Py_BuildValue("sh",
    + ifname,
    + a->can_family);

    1. a->can_family would be better as a 'i' to be consistent with the
      rest of the code

    2. you should use the FS encoding for the interface name, e.g.

              return Py_BuildValue("O&i",
                                   PyUnicode_DecodeFSDefault
                                   ifname,
                                   a->can_family);
      

    (you can have a look at socket_if_nameindex for an example).

    @@ -1486,6 +1505,43 @@ getsockaddrarg(PySocketSockObject *s, Py

                       if (!PyArg_ParseTuple(args, "s", &interfaceName))
                               return 0;
    
    1. same thing here, you should use
      if (!PyArg_ParseTuple(args, "O&", PyUnicode_FSConverter,
      &interfaceName))
      return 0;

    (see socket_if_nametoindex for an example)

    @@ -1486,6 +1505,43 @@ getsockaddrarg(PySocketSockObject *s, Py

    + if (!strcmp(interfaceName, "any")) {
    + ifr.ifr_ifindex = 0;

    To be consistent with the convention used IPv4/IPv6
    INADDR_ANY/in6addr_any, I would prefer if you replaced the any
    interface name by "" (empty string)

    @@ -69,6 +69,20 @@ typedef int socklen_t;

    +#ifdef HAVE_LINUX_CAN_H
    +#include <linux/can.h>
    +#ifndef PF_CAN
    +# define PF_CAN 29
    +#endif
    +#ifndef AF_CAN
    +# define AF_CAN PF_CAN
    +#endif
    +#endif

    I don't see how PF_CAN or AF_CAN could not be defined lin <linux/can.h>.
    And if they're not defined, it's probably not a good idea to define
    them arbitrarily...

    +#ifdef HAVE_LINUX_CAN_H
    + struct sockaddr_can* can;
    +#endif

    it should be struct sockaddr_can can here, not a pointer.

    • could you add a test for SocketCan in Lib/test/socket.py ?
      It would be nice to have a basic test which can be run on all the
      kernels supporting PF_CAN (typically just creating a socket, binding
      to it and closing it), and another more complete test if there's a CAN
      interface (sending and receiving a packet, probably using the
      loopback).
      Take into account that SOCK_RAW are priviledged and restricted to root
      or CAP_SYS_RAW.

    @ogait87
    Copy link
    Mannequin

    ogait87 mannequin commented Jul 20, 2011

    This patch should be easy to verify and includes documentation and the test cases.

    ogait87@mypc:~/cpython$ hg sum
    parent: 71435:6f9d917df541 tip
    Fix test_multiprocessing failure under Windows.
    branch: default
    commit: 7 modified
    update: (current)

    @neologix
    Copy link
    Mannequin

    neologix mannequin commented Jul 24, 2011

    Thanks for updating your patch.
    I've done a review, available here:
    http://bugs.python.org/review/10141/show

    @ogait87
    Copy link
    Mannequin

    ogait87 mannequin commented Aug 1, 2011

    I think that this time i have included almost every proposed changes.

    For the unitTest I did not included the condition "os.getuid() == 0" because of this information in the Linux documentation "3.3 network security issues (capabilities)" http://www.mjmwired.net/kernel/Documentation/networking/can.txt#212. For example, the Ubuntu 10.10 does not require to be root to access socketCAN network interfaces.

    For the "return Py_BuildValue("O&h", PyUnicode_DecodeFSDefault, ifname, a->can_family);" i have kept the "h" because in the header file the "a->can_family" is defined as an short int.

    @neologix
    Copy link
    Mannequin

    neologix mannequin commented Sep 22, 2011

    Here's an updated patch, with more tests.
    Please review!

    @vstinner
    Copy link
    Member

    socketcan_v4.patch:

    • dummy question: why an address is a tuple with 1 string instead of just the string? Does AF_UNIX also uses a tuple of 1 string?
    • the example should also use struct.pack() to create the frame, I don't like hardcoded BLOB
    • in test_socket: _have_socket_can() interprets permission denied as "CAN is not supported", it would be nice to provide a better skip message. Create maybe a decorator based?
    • _have_socket_can(): you may move s.close() outside the try block (add maybe a "else:" block?) because you may hide a real bug in .close()
    • data += b'\0' * (8 - can_dlc): I prefer data = data.ljust(8, '\x00')
    • you might add frame encoder/decoder in your example
    • if (!strcmp(PyBytes_AS_STRING(interfaceName), "")) hum..... PyBytes_GET_SIZE(intername)==0 should be enough
    • you truncate the interface name, it can be surprising, I would prefer an error (e.g. "interface name too long: 20 characters, the maximum is 10 characters" ?)
    • (oh no! don't include horrible configure diff in patches for the bug tracker :-p)

    In which Linux version was CAN introduced?

    @neologix
    Copy link
    Mannequin

    neologix mannequin commented Sep 23, 2011

    • dummy question: why an address is a tuple with 1 string instead of just
      the string? Does AF_UNIX also uses a tuple of 1 string?
    I think the reason behind the tuple is future proofing.
    Here's the definition of `struct sockaddr_can` in my Linux box's headers:
    """
    /**
     * struct sockaddr_can - the sockaddr structure for CAN sockets
     * @can_family:  address family number AF_CAN.
     * @can_ifindex: CAN network interface index.
     * @can_addr:    protocol specific address information
     */
    struct sockaddr_can {
        sa_family_t can_family;
        int         can_ifindex;
        union {
            /* transport protocol class address information (e.g. ISOTP) */
            struct { canid_t rx_id, tx_id; } tp;
        /* reserved for future CAN protocols address information \*/
    } can_addr;
    

    };
    """

    By making it a tuple, it will be easier to extend the address that
    must be passed to bind(2), should it ever evolve, in a backward
    compatible way. Well, that's just a guess (I'm by no means a SocketCAN
    expert :-).

    • the example should also use struct.pack() to create the frame, I don't
      like hardcoded BLOB

    Done.

    • in test_socket: _have_socket_can() interprets permission denied as "CAN
      is not supported", it would be nice to provide a better skip message. Create
      maybe a decorator based?

    AFAICT, it shouldn't fail with EPERM or so.
    Also, I'm not sure what the message would look like, and it's probably
    a bit overkill.

    • _have_socket_can(): you may move s.close() outside the try block (add
      maybe a "else:" block?) because you may hide a real bug in .close()

    Changed that.

    • data += b'\0' * (8 - can_dlc): I prefer data = data.ljust(8, '\x00')

    Hum... Done.

    • you might add frame encoder/decoder in your example

    Done.

    • if (!strcmp(PyBytes_AS_STRING(interfaceName), "")) hum.....
      PyBytes_GET_SIZE(intername)==0 should be enough

    Done.

    • you truncate the interface name, it can be surprising, I would prefer an
      error (e.g. "interface name too long: 20 characters, the maximum is 10
      characters" ?)

    I changed that, and added a test. Also, note that AF_PACKET suffers
    from the same problem.
    I'll submit a separate patch.

    • (oh no! don't include horrible configure diff in patches for the bug
      tracker :-p)

    Yeah, I usually take care of that, but forgot this time.

    In which Linux version was CAN introduced?

    Apparently, 2.6.25. Note that we don't need
    @support.requires_linux_version() though, it should be catched by
    HAVE_SOCKET_CAN (also, you can't use it as a class decorator...).

    Here's the updated patch. It passes on all the buildbots (of course,
    it's only relevant on Linux).

    @neologix
    Copy link
    Mannequin

    neologix mannequin commented Oct 2, 2011

    So, Victor, what do you think of the last version?
    This patch has been lingering for quite some time, and it's really a
    cool feature.

    @pitrou
    Copy link
    Member

    pitrou commented Oct 5, 2011

    I don't have much to say about the patch, given that I don't know anything about CAN and my system doesn't appear to have a "vcan0" interface. I think it's ok to commit and refine later if something turns out insufficient.

    @neologix
    Copy link
    Mannequin

    neologix mannequin commented Oct 5, 2011

    I don't have much to say about the patch, given that I don't know
    anything about CAN and my system doesn't appear to have a "vcan0"
    interface.

    I had never heard about it before this issue, but the protocol is really simple.

    If you want to try it out (just for fun :-), you just have to do the following:
    # modprobe vcan
    # ip link add dev vcan0 type vcan
    # ifconfig vcan0 up

    @pitrou
    Copy link
    Member

    pitrou commented Oct 5, 2011

    I had never heard about it before this issue, but the protocol is really simple.

    If you want to try it out (just for fun :-), you just have to do the following:

    modprobe vcan

    ip link add dev vcan0 type vcan

    ifconfig vcan0 up

    Ah, thanks! Can you add a comment about that in test_socket.py?
    I can confirm that all tests pass ok on my Linux system (kernel
    2.6.38.8-desktop-5.mga).

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Oct 6, 2011

    New changeset e767318baccd by Charles-François Natali in branch 'default':
    Issue bpo-10141: socket: add SocketCAN (PF_CAN) support. Initial patch by Matthias
    http://hg.python.org/cpython/rev/e767318baccd

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Oct 6, 2011

    New changeset a4af684bb54e by Victor Stinner in branch 'default':
    Issue bpo-10141: Don't use hardcoded frame size in example, use struct.calcsize()
    http://hg.python.org/cpython/rev/a4af684bb54e

    @neologix
    Copy link
    Mannequin

    neologix mannequin commented Oct 6, 2011

    Committed.
    Matthias, Tiago, thanks!

    @neologix neologix mannequin closed this as completed Oct 6, 2011
    @neologix
    Copy link
    Mannequin

    neologix mannequin commented Oct 7, 2011

    From python-dev:
    """
    I work on Ubuntu Jaunty for my cpython development work - an old version, I
    know, but still quite serviceable and has worked well for me over many months.
    With the latest default cpython repository, however, I can't run the regression
    suite because the socket module now fails to build:

    gcc -pthread -fPIC -g -O0 -Wall -Wstrict-prototypes -IInclude -I.
    -I./Include -I/usr/local/include -I/home/vinay/projects/python/default -c
    /home/vinay/projects/python/default/Modules/socketmodule.c
    -o build/temp.linux-i686-3.3-pydebug/home/vinay/projects/python/default
    /Modules/socketmodule.o
    .../Modules/socketmodule.c: In function
    ‘makesockaddr’:
    .../Modules/socketmodule.c:1224: error: ‘AF_CAN’ undeclared (first use in
    this function)
    .../Modules/socketmodule.c:1224: error: (Each undeclared identifier is
    reported only once
    .../Modules/socketmodule.c:1224: error: for each function it appears in.)
    .../Modules/socketmodule.c: In function
    ‘getsockaddrarg’:
    .../Modules/socketmodule.c:1610: error: ‘AF_CAN’ undeclared (first use in
    this function)
    .../Modules/socketmodule.c: In function ‘getsockaddrlen’:
    .../Modules/socketmodule.c:1750: error: ‘AF_CAN’ undeclared (first use in
    this function)

    On this system, AF_CAN *is* defined, but in linux/socket.h, not in sys/socket.h.
    From what I can see, sys/socket.h includes bits/socket.h which includes
    asm/socket.h, but apparently linux/socket.h isn't included.
    """

    Vinay, what happens if you replace in Modules/socketmodule.h:
    """
    #ifdef HAVE_LINUX_CAN_H
    #include <linux/can.h>
    #endif
    """

    with

    """
    #ifdef HAVE_LINUX_CAN_H
    #include <linux/socket.h>
    #include <linux/can.h>
    #endif
    """

    @neologix neologix mannequin reopened this Oct 7, 2011
    @vsajip
    Copy link
    Member

    vsajip commented Oct 7, 2011

    I added the line in the suggested place:

    #ifdef HAVE_LINUX_TIPC_H
    # include <linux/tipc.h>
    #endif
    
    #ifdef HAVE_LINUX_CAN_H
    #include <linux/socket.h>  /* the line I added - line 76 */
    #include <linux/can.h>
    #endif
    
    #ifdef HAVE_LINUX_CAN_RAW_H
    #include <linux/can/raw.h>
    #endif

    Strangely, it seems to make no difference! I copied out the actual gcc line from the make file and ran it from the command line to just pre-process:

    gcc -pthread -fPIC -g -O0 -Wall -Wstrict-prototypes -IInclude -I. -I./Include -I/usr/local/include -I/home/vinay/projects/python/default -E /home/vinay/projects/python/default/Modules/socketmodule.c >/tmp/tmp.c

    (I assume /usr/include is always searched, as it's not explicitly named in a -I parameter.)

    Looking at the tmp.c file produced, here's the relevant section:

    # 182 "/usr/include/linux/tipc.h" 3 4
    struct sockaddr_tipc {
     unsigned short family;
     unsigned char addrtype;
     signed char scope;
     union {
      struct tipc_portid id;
      struct tipc_name_seq nameseq;
      struct {
       struct tipc_name name;
       __u32 domain;
      } name;
     } addr;
    };
    # 73 "/home/vinay/projects/python/default/Modules/socketmodule.h" 2

    # 1 "/usr/include/linux/can.h" 1 3 4

    After the tipc.h include, the can.h include comes next, as if I hadn't added the linux/socket.h line at all! What is this I don't even ...

    I pasted the whole socketmodule.h file to a gist here:

    https://gist.github.com/1270436

    Tell me I'm not going mad!

    @vsajip
    Copy link
    Member

    vsajip commented Oct 7, 2011

    Just to test, I added the full absolute path name in socketmodule.h:

    #ifdef HAVE_LINUX_CAN_H
    #include "/usr/include/linux/socket.h"
    #include <linux/can.h>
    #endif

    Now, the snippet from pre-processing is:

    # 182 "/usr/include/linux/tipc.h" 3 4
    struct sockaddr_tipc {
     unsigned short family;
     unsigned char addrtype;
     signed char scope;
     union {
      struct tipc_portid id;
      struct tipc_name_seq nameseq;
      struct {
       struct tipc_name name;
       __u32 domain;
      } name;
     } addr;
    };
    # 73 "/home/vinay/projects/python/default/Modules/socketmodule.h" 2

    # 1 "/usr/include/linux/socket.h" 1
    # 77 "/home/vinay/projects/python/default/Modules/socketmodule.h" 2
    # 1 "/usr/include/linux/can.h" 1 3 4
    # 41 "/usr/include/linux/can.h" 3 4

    This implies that the linux/socket.h file is not being read at all. First part of linux/socket.h is:

    #ifndef _LINUX_SOCKET_H
    #define _LINUX_SOCKET_H
    

    @vsajip
    Copy link
    Member

    vsajip commented Oct 7, 2011

    Ok, I found that linux/socket.h *is* actually included earlier, from /usr/include/linux/netlink.h. However, the AF_CAN definition appears only under the condition

    #if defined(__KERNEL__) || !defined(__GLIBC__) || (__GLIBC__ < 2)
    
    ...
    #define AF_CAN		29	/* Controller Area Network      */
    ...

    #endif /* not kernel and not glibc */
    #endif /* _LINUX_SOCKET_H */

    which would imply that on this system at least, the AF_CAN definition is supposed to come from elsewhere. I did a recursive grep in /usr/include: the only place AF_CAN is defined is linux/socket.h.

    @neologix
    Copy link
    Mannequin

    neologix mannequin commented Oct 7, 2011

    which would imply that on this system at least, the AF_CAN definition is
    supposed to come from elsewhere.

    Yes, from <bits/socket.h>.
    Looks like a crappy libc version: <linux/can.h> is present, but AF_CAN is not defined.
    Just for fun, is PF_CAN defined?

    You might try the following in configure.in:
    """
    # On Linux, can.h and can/raw.h require sys/socket.h
    AC_CHECK_HEADERS(linux/can.h linux/can/raw.h,,,[
    #ifdef HAVE_SYS_SOCKET_H
    #include <sys/socket.h>
    #ifndef AF_CAN
    # error "AF_CAN not defined"
    #endif
    #endif
    ])
    """

    @vsajip
    Copy link
    Member

    vsajip commented Oct 7, 2011

    PF_CAN is defined by

    #define PF_CAN		AF_CAN

    in linux/socket.h :-(

    Making the change in configure.in didn't lead to any change: no error when I ran configure (which is ./configure --with-py-debug in my case), and when I build, the same error (AF_CAN undefined) occurs. Just to eliminate any extraneous variables and be absolutely sure, the program

    #include <sys/socket.h>
    #include <stdio.h>
    
    int main(int argc, char ** argv)
    {
        printf("AF_CAN is %d\n", AF_CAN);
    }

    also fails with the same error.

    @pitrou
    Copy link
    Member

    pitrou commented Oct 7, 2011

    Making the change in configure.in didn't lead to any change: no error
    when I ran configure

    Did you run autoconf (or autoreconf) before?

    @vsajip
    Copy link
    Member

    vsajip commented Oct 7, 2011

    Did you run autoconf (or autoreconf) before?

    D'oh! I didn't realise I had to, though in retrospect it should have been obvious ... autoconf isn't even installed on my system, and I can't install it at the moment because of some problem with the Ubuntu repos for Jaunty.

    @neologix
    Copy link
    Mannequin

    neologix mannequin commented Oct 7, 2011

    Here's a better patch.
    It only touches socketmodule.c, so no need for autoconf, just use make.
    If it works on your box, I'll test it on the custom buildbots before pushing it for good (if I learned one thing, it's to never underestimate the potential for broken headers/libc).

    @vsajip
    Copy link
    Member

    vsajip commented Oct 7, 2011

    That did the trick - I can now build the socket module. Thanks!

    I noticed that the module initialisation code uses #ifdef AF_CAN and #ifdef PF_CAN rather than #ifdef HAVE_LINUX_CAN_H :-)

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Oct 7, 2011

    New changeset d4ce850b06b7 by Charles-François Natali in branch 'default':
    Issue bpo-10141: fix socketmodule compilation on Linux systems with <linux/can.h>
    http://hg.python.org/cpython/rev/d4ce850b06b7

    @neologix
    Copy link
    Mannequin

    neologix mannequin commented Oct 8, 2011

    Working fine on the buildbots and Vinay's box, closing!

    @neologix neologix mannequin closed this as completed Oct 8, 2011
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Mar 20, 2014

    New changeset df427bf067d7 by Vinay Sajip in branch '3.4':
    Issue bpo-10141: updated new usages of AF_CAN to be in #ifdef AF_CAN rather than #ifdef HAVE_LINUX_CAN_H to allow compilation on older Linuxes.
    http://hg.python.org/cpython/rev/df427bf067d7

    @neologix
    Copy link
    Mannequin

    neologix mannequin commented Mar 20, 2014

    Vinay, your change just reverted this: http://bugs.python.org/issue20065

    Basically, AF_CAN being defined doesn't imply that CAN_RAW is defined.

    So compilation will now fail - again - on those hosts.

    @neologix neologix mannequin reopened this Mar 20, 2014
    @vsajip
    Copy link
    Member

    vsajip commented Mar 20, 2014

    Sorry if I messed up - I just applied the same logic as I thought you had done earlier (replacing #ifdef HAVE_LINUX_CAN_H with #ifdef AF_CAN), and everything compiled OK after my changes.

    Are you saying that an additional clause for CAN_RAW being defined should be added around where it is used? Would that sort things out? I'd rather not just revert my change, as that would mean I couldn't compile the SSL module.

    @neologix
    Copy link
    Mannequin

    neologix mannequin commented Mar 21, 2014

    Are you saying that an additional clause for CAN_RAW being defined should
    be added around where it is used? Would that sort things out?

    Yes.

    I'd rather not just revert my change, as that would mean I couldn't
    compile the SSL module.

    I don't get it: how could the previous code prevent the SSL module from
    being built?
    What error were you getting?

    @vsajip
    Copy link
    Member

    vsajip commented Mar 21, 2014

    What error were you getting?

    That AF_CAN was undefined (even though HAVE_LINUX_CAN_H is). This is on Ubuntu Jaunty, which I use for my Python core development.

    Note the change you made in d4ce850b06b7 to fix this problem when it occurred before - it's the same approach as my recent change that we're talking about. It seems that in 3.4, some more instances of AF_CAN usage were added, but wrapped in #ifdef HAVE_LINUX_CAN_H, which led to the new compilation failures.

    I'll make the changes to take CAN_RAW into account.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Mar 21, 2014

    New changeset 9dc199b921eb by Vinay Sajip in branch '3.4':
    Issue bpo-10141, bpo-20065: Changed #if to take CAN_RAW into account.
    http://hg.python.org/cpython/rev/9dc199b921eb

    New changeset 20cced06acdd by Vinay Sajip in branch 'default':
    Issue bpo-10141, bpo-20065: Merged from 3.4.
    http://hg.python.org/cpython/rev/20cced06acdd

    @neologix
    Copy link
    Mannequin

    neologix mannequin commented Mar 21, 2014

    That AF_CAN was undefined (even though HAVE_LINUX_CAN_H is). This is on
    Ubuntu Jaunty, which I use for my Python core development.

    How dear...

    The latest change should be OK.

    @vadmium
    Copy link
    Member

    vadmium commented Dec 2, 2015

    Sounds like the last problem has been fixed, so can we close this?

    @vsajip vsajip closed this as completed Dec 4, 2015
    @xrmx
    Copy link
    Mannequin

    xrmx mannequin commented Jul 5, 2017

    I have an issue related to this while trying to compile statically Python 3.6.1 against a static musl.

    The problem is that i have AF_CAN defined because it's defined in linux/socket.h but by not having HAVE_LINUX_CAN_H defined in pyconfig.h the header which contains the definition of struct sockaddr_can is not included. So i think (at least for linux) using AF_CAN for the conditionals is wrong and the HAVE_LINUX_CAN_H should be used instead.

    I think the same applies for CAN_RAW and CAN_BCM because they are defined in the generic linux/can.h and not in a feature specific header.

    @bitdancer
    Copy link
    Member

    This issue is closed. Please open a new issue for your problem and proposal.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    topic-IO type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    7 participants