From 5d0b8b4ac73d434160c1b71a7922ad7309082a46 Mon Sep 17 00:00:00 2001 From: Uwe Kleine-König Date: Fri, 14 Jun 2019 17:22:55 +0200 Subject: Replace reading and writing to a device by a ios specific callback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is a restructuring without any intended side effect. The motivation is that this way telnet handling can go to telnet.c and so only affect telnet connections. Similar for can this should allow to get rid of the pipe and pthread stuff. Signed-off-by: Uwe Kleine-König --- can.c | 12 ++++++++++++ microcom.h | 2 ++ mux.c | 42 +++++++++++++++++++++++++++++++++++++----- serial.c | 12 ++++++++++++ telnet.c | 12 ++++++++++++ 5 files changed, 75 insertions(+), 5 deletions(-) diff --git a/can.c b/can.c index 1de2a12..38e8e96 100644 --- a/can.c +++ b/can.c @@ -49,6 +49,16 @@ struct can_data { static struct can_data data; static pthread_t can_thread; +static ssize_t can_write(struct ios_ops *ios, const void *buf, size_t count) +{ + return write(ios->fd, buf, count); +} + +static ssize_t can_read(struct ios_ops *ios, void *buf, size_t count) +{ + return read(ios->fd, buf, count); +} + static int can_set_speed(struct ios_ops *ios, unsigned long speed) { return 0; @@ -154,6 +164,8 @@ struct ios_ops *can_init(char *interface_id) if (!ios) return NULL; + ios->write = can_write; + ios->read = can_read; ios->set_speed = can_set_speed; ios->set_flow = can_set_flow; ios->send_break = can_send_break; diff --git a/microcom.h b/microcom.h index e8ed544..fbbb96b 100644 --- a/microcom.h +++ b/microcom.h @@ -38,6 +38,8 @@ #define DEFAULT_CAN_ID (0x200) struct ios_ops { + ssize_t (*write)(struct ios_ops *, const void *buf, size_t count); + ssize_t (*read)(struct ios_ops *, void *buf, size_t count); int (*set_speed)(struct ios_ops *, unsigned long speed); #define FLOW_NONE 0 #define FLOW_SOFT 1 diff --git a/mux.c b/mux.c index 5dab65d..fb56a1d 100644 --- a/mux.c +++ b/mux.c @@ -20,6 +20,7 @@ #include "microcom.h" #include #include +#include #include #define BUFSIZE 1024 @@ -219,6 +220,37 @@ static void write_receive_buf(const unsigned char *buf, int len) write(logfd, buf, len); } +static int ios_printf(struct ios_ops *ios, const char *format, ...) +{ + char buf[20]; + int size, written = 0; + ssize_t ret; + va_list args; + + va_start(args, format); + + size = vsnprintf(buf, sizeof(buf), format, args); + + va_end(args); + + if (size >= sizeof(buf)) { + /* truncated output */ + errno = EIO; + return -1; + } + + while (written < size) { + ret = ios->write(ios, buf + written, size - written); + if (ret < 0) + return ret; + + written += ret; + assert(written <= size); + } + + return written; +} + /* This function is called with buf[0] being IAC. */ static int handle_command(struct ios_ops *ios, unsigned char *buf, int len) { @@ -255,7 +287,7 @@ static int handle_command(struct ios_ops *ios, unsigned char *buf, int len) } else { /* unknown/unimplemented option -> DONT */ dbg_printf(" -> DONT\n"); - dprintf(ios->fd, "%c%c%c", IAC, DONT, buf[2]); + ios_printf(ios, "%c%c%c", IAC, DONT, buf[2]); } return 3; @@ -283,7 +315,7 @@ static int handle_command(struct ios_ops *ios, unsigned char *buf, int len) } else { /* Oh, cannot handle that one, so send a WONT */ dbg_printf(" -> WONT\n"); - dprintf(ios->fd, "%c%c%c", IAC, WONT, buf[2]); + ios_printf(ios, "%c%c%c", IAC, WONT, buf[2]); } return 3; @@ -322,7 +354,7 @@ static int handle_receive_buf(struct ios_ops *ios, unsigned char *buf, int len) case 5: write_receive_buf(sendbuf, buf - sendbuf); if (answerback) - write(ios->fd, answerback, strlen(answerback)); + ios->write(ios, answerback, strlen(answerback)); else write_receive_buf(buf, 1); @@ -353,7 +385,7 @@ static void cook_buf(struct ios_ops *ios, unsigned char *buf, int num) current++; /* and write the sequence before esc char to the comm port */ if (current) - write(ios->fd, buf, current); + ios->write(ios, buf, current); if (current < num) { /* process an escape sequence */ /* found an escape character */ @@ -411,7 +443,7 @@ int mux_loop(struct ios_ops *ios) if (FD_ISSET(ios->fd, &ready)) { /* pf has characters for us */ - len = read(ios->fd, buf, BUFSIZE); + len = ios->read(ios, buf, BUFSIZE); if (len < 0) { ret = -errno; fprintf(stderr, "%s\n", strerror(-ret)); diff --git a/serial.c b/serial.c index 0778fe1..1540907 100644 --- a/serial.c +++ b/serial.c @@ -48,6 +48,16 @@ static void init_comm(struct termios *pts) pts->c_iflag &= ~ICRNL; } +static ssize_t serial_write(struct ios_ops *ios, const void *buf, size_t count) +{ + return write(ios->fd, buf, count); +} + +static ssize_t serial_read(struct ios_ops *ios, void *buf, size_t count) +{ + return read(ios->fd, buf, count); +} + static int serial_set_handshake_line(struct ios_ops *ios, int pin, int enable) { int flag; @@ -158,6 +168,8 @@ struct ios_ops * serial_init(char *device) if (!lockfile) return NULL; + ops->write = serial_write; + ops->read = serial_read; ops->set_speed = serial_set_speed; ops->set_flow = serial_set_flow; ops->set_handshake_line = serial_set_handshake_line; diff --git a/telnet.c b/telnet.c index fe30da7..4726a98 100644 --- a/telnet.c +++ b/telnet.c @@ -26,6 +26,16 @@ #include "microcom.h" +static ssize_t telnet_write(struct ios_ops *ios, const void *buf, size_t count) +{ + return write(ios->fd, buf, count); +} + +static ssize_t telnet_read(struct ios_ops *ios, void *buf, size_t count) +{ + return read(ios->fd, buf, count); +} + static int telnet_set_speed(struct ios_ops *ios, unsigned long speed) { @@ -92,6 +102,8 @@ struct ios_ops *telnet_init(char *hostport) if (!ios) return NULL; + ios->write = telnet_write; + ios->read = telnet_read; ios->set_speed = telnet_set_speed; ios->set_flow = telnet_set_flow; ios->send_break = telnet_send_break; -- cgit v1.2.3 From dc0d7aad38f21629074416c2b2e8a5cf9c269785 Mon Sep 17 00:00:00 2001 From: Uwe Kleine-König Date: Fri, 14 Jun 2019 18:01:21 +0200 Subject: can: Get rid of pthread for can transfers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since there are ios specific read and write callbacks the needed overhead for can transfers can be implemented in these which makes the separate thread superfluous. This simplifies the can driver considerably. As a consequence microcom doesn't need pthreads any more and the respective changes in configure.ac can go away, too. The configuration for lgtm which assert that the AX_PTHREAD macro is available can be dropped and the travis configuration can be simplified accordingly. Signed-off-by: Uwe Kleine-König Closes: https://github.com/pengutronix/microcom/issues/8 --- .travis.yml | 4 -- can.c | 137 ++++++++++++++++++++--------------------------------------- configure.ac | 6 +-- lgtm.yml | 5 --- 4 files changed, 47 insertions(+), 105 deletions(-) delete mode 100644 lgtm.yml diff --git a/.travis.yml b/.travis.yml index 87116f4..0723a3a 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,10 +1,6 @@ language: c compiler: gcc dist: trusty -addons: - apt: - packages: - - autoconf-archive script: - autoreconf -fi - ./configure diff --git a/can.c b/can.c index 38e8e96..e64127b 100644 --- a/can.c +++ b/can.c @@ -15,7 +15,6 @@ * */ -#include #include #include #include @@ -34,29 +33,59 @@ #include "microcom.h" -enum socket_local_enum { - SOCKET_CANSIDE, - SOCKET_TERMSIDE, - SOCKET_MAX, -}; - struct can_data { - int socket_can; - int socket_local[SOCKET_MAX]; int can_id; }; static struct can_data data; -static pthread_t can_thread; static ssize_t can_write(struct ios_ops *ios, const void *buf, size_t count) { - return write(ios->fd, buf, count); + size_t loopcount; + ssize_t ret = 0, err; + + struct can_frame to_can = { + .can_id = data.can_id, + }; + + while (count > 0) { + loopcount = min(count, sizeof(to_can.data)); + memcpy(to_can.data, buf, loopcount); + to_can.can_dlc = loopcount; +retry: + err = write(ios->fd, &to_can, sizeof(to_can)); + if (err < 0 && errno == EINTR) + goto retry; + + if (err < 0) + return err; + + assert(err < count); + buf += err; + count -= err; + ret += err; + } + + return ret; } static ssize_t can_read(struct ios_ops *ios, void *buf, size_t count) { - return read(ios->fd, buf, count); + struct can_frame from_can; + ssize_t ret; + +retry: + ret = read(ios->fd, &from_can, sizeof(from_can)); + if (ret < 0 && errno != EINTR) + goto retry; + + if (ret < 0) + return ret; + + assert(count >= from_can.can_dlc); + memcpy(buf, from_can.data, from_can.can_dlc); + + return from_can.can_dlc; } static int can_set_speed(struct ios_ops *ios, unsigned long speed) @@ -76,75 +105,10 @@ static int can_send_break(struct ios_ops *ios) static void can_exit(struct ios_ops *ios) { - shutdown(ios->fd, SHUT_RDWR); close(ios->fd); - pthread_join(can_thread, NULL); free(ios); } -static void *can_thread_fun(void *_data) -{ - struct can_data *data = _data; - struct can_frame to_can = { - .can_id = data->can_id, - }; - struct can_frame from_can; - fd_set ready; - int ret, fd_max; - - fd_max = max(data->socket_can, data->socket_local[SOCKET_CANSIDE]); - fd_max++; - - while (1) { - FD_ZERO(&ready); - FD_SET(data->socket_can, &ready); - FD_SET(data->socket_local[SOCKET_CANSIDE], &ready); - - ret = select(fd_max, &ready, NULL, NULL, NULL); - if (ret == -1) { - if (errno == EINTR) - continue; - goto exit; - } - - /* CAN -> TERMINAL */ - if (FD_ISSET(data->socket_can, &ready)) { - ret = read(data->socket_can, &from_can, sizeof(from_can)); - if ((ret == -1 && errno != EINTR) || - ret != sizeof(from_can)) - goto exit; - - ret = write(data->socket_local[SOCKET_CANSIDE], - from_can.data, from_can.can_dlc); - if ((ret == -1 && errno != EINTR) || - ret != from_can.can_dlc) - goto exit; - } - - /* TERMINAL -> CAN */ - if (FD_ISSET(data->socket_local[SOCKET_CANSIDE], &ready)) { - ret = read(data->socket_local[SOCKET_CANSIDE], - to_can.data, sizeof(to_can.data)); - if ((ret == -1 && errno != EINTR) || - ret == 0) - goto exit; - - to_can.can_dlc = ret; - ret = write(data->socket_can, &to_can, sizeof(to_can)); - if ((ret == -1 && errno != EINTR) || - ret != sizeof(to_can)) - goto exit; - } - } - - exit: - shutdown(data->socket_local[SOCKET_CANSIDE], SHUT_RDWR); - close(data->socket_local[SOCKET_CANSIDE]); - close(data->socket_can); - - return NULL; -} - struct ios_ops *can_init(char *interface_id) { struct ios_ops *ios; @@ -201,39 +165,30 @@ struct ios_ops *can_init(char *interface_id) /* no cleanups on failure, we exit anyway */ - data.socket_can = socket(PF_CAN, SOCK_RAW, CAN_RAW); - if (data.socket_can < 0) { + ios->fd = socket(PF_CAN, SOCK_RAW, CAN_RAW); + if (ios->fd < 0) { perror("socket"); return NULL; } - if (setsockopt(data.socket_can, SOL_CAN_RAW, CAN_RAW_FILTER, + if (setsockopt(ios->fd, SOL_CAN_RAW, CAN_RAW_FILTER, filter, sizeof(filter))) { perror("setsockopt"); return NULL; } strcpy(ifr.ifr_name, interface); - if (ioctl(data.socket_can, SIOCGIFINDEX, &ifr)) { + if (ioctl(ios->fd, SIOCGIFINDEX, &ifr)) { printf("%s: %s\n", interface, strerror(errno)); return NULL; } addr.can_ifindex = ifr.ifr_ifindex; - if (bind(data.socket_can, (struct sockaddr *)&addr, sizeof(addr)) < 0) { + if (bind(ios->fd, (struct sockaddr *)&addr, sizeof(addr)) < 0) { perror("bind"); return NULL; } - if (socketpair(AF_UNIX, SOCK_STREAM, 0, data.socket_local) < 0) { - perror("socketpair"); - return NULL; - } - - if (pthread_create(&can_thread, NULL, can_thread_fun, &data) != 0) - return NULL; - - ios->fd = data.socket_local[SOCKET_TERMSIDE]; printf("connected to %s (rx_id=%x, tx_id=%x)\n", interface, filter->can_id, data.can_id); diff --git a/configure.ac b/configure.ac index 18b4346..d1a90b6 100644 --- a/configure.ac +++ b/configure.ac @@ -9,7 +9,6 @@ AC_CONFIG_HEADERS([config.h]) # Checks for programs. AC_PROG_CC -AX_PTHREAD AC_SYS_LARGEFILE # Checks for libraries. @@ -38,10 +37,7 @@ AS_IF([test "x$enable_can" != "xno"], ]) AS_IF([test "x$enable_can" = "xyes"], - [AC_DEFINE([USE_CAN], [1], [Define if can mode should be built-in]) - LIBS="$PTHREAD_LIBS $LIBS" - CFLAGS="$CFLAGS $PTHREAD_CFLAGS" - CC="$PTHREAD_CC"]) + [AC_DEFINE([USE_CAN], [1], [Define if can mode should be built-in])]) AM_CONDITIONAL([CAN], [test "x$enable_can" = "xyes"]) diff --git a/lgtm.yml b/lgtm.yml deleted file mode 100644 index 3c21140..0000000 --- a/lgtm.yml +++ /dev/null @@ -1,5 +0,0 @@ -extraction: - cpp: - prepare: - packages: - - autoconf-archive -- cgit v1.2.3