Charles-François Natali <neolo...@free.fr> added the comment:
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;
3) same thing here, you should use
if (!PyArg_ParseTuple(args, "O&", PyUnicode_FSConverter,
&interfaceName))
return 0;
(see socket_if_nametoindex for an example)
4)
@@ -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)
5)
@@ -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...
6)
+#ifdef HAVE_LINUX_CAN_H
+ struct sockaddr_can* can;
+#endif
it should be struct sockaddr_can can here, not a pointer.
7)
- you should update Doc/library/socket.rst
8)
- 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.
----------
_______________________________________
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue10141>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
