From 3001dda198c1c534ae7f37134466057a306a7e1a Mon Sep 17 00:00:00 2001 From: Venkatesh Shukla Date: Tue, 13 May 2014 15:00:48 +0530 Subject: [PATCH 1/3] Minor error in serial_configure Due to a minor mistake, only the first byte was being checked for equality. Fixed it by changing position of parenthesis. Signed-off-by: Venkatesh Shukla --- src/serial_posix.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/serial_posix.c b/src/serial_posix.c index e56eb0d..80334b0 100644 --- a/src/serial_posix.c +++ b/src/serial_posix.c @@ -426,7 +426,7 @@ serial_configure (serial_t *device, int baudrate, int databits, int parity, int SYSERROR (device->context, errno); return -1; } - if (memcmp (&tty, &active, sizeof (struct termios) != 0)) { + if (memcmp (&tty, &active, sizeof (struct termios)) != 0) { ERROR (device->context, "Failed to set the terminal attributes."); return -1; } From 5f1a18653d0bd671c1df6ccfa08e99c66a0e7ad1 Mon Sep 17 00:00:00 2001 From: Jef Driesen Date: Wed, 14 May 2014 09:52:34 +0200 Subject: [PATCH 2/3] Initialize the termios structure. The previous commit exposed another issue. The termios structure may contain padding bytes. Because the content of those padding bytes is unspecified, they may contain some random data, which causes the memcmp to fail. Explicitly initializing the termios structure with memset, will also set the padding bytes to zero. --- src/serial_posix.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/serial_posix.c b/src/serial_posix.c index 80334b0..52e9191 100644 --- a/src/serial_posix.c +++ b/src/serial_posix.c @@ -233,6 +233,7 @@ serial_configure (serial_t *device, int baudrate, int databits, int parity, int // Retrieve the current settings. struct termios tty; + memset (&tty, 0, sizeof (tty)); if (tcgetattr (device->fd, &tty) != 0) { SYSERROR (device->context, errno); return -1; @@ -422,6 +423,7 @@ serial_configure (serial_t *device, int baudrate, int databits, int parity, int // tcgetattr() to check that all changes have been performed successfully. struct termios active; + memset (&active, 0, sizeof (active)); if (tcgetattr (device->fd, &active) != 0) { SYSERROR (device->context, errno); return -1; From 197b9f09421111e03588c94d55a72aa6ec624c63 Mon Sep 17 00:00:00 2001 From: Jef Driesen Date: Wed, 28 May 2014 06:29:34 +0200 Subject: [PATCH 3/3] Remove the extra check of the termios structure. The extra memcmp check after the tcsetattr call is intended to verify whether all the changes to the termios structure have been applied correctly. But for pty's, some of the termios settings make no sense at all, and therefore the Linux kernel always does: tty->termios.c_cflag &= ~(CSIZE | PARENB); tty->termios.c_cflag |= (CS8 | CREAD); Thus, instead of ignoring such nonsense termios settings, the kernel changes the termios structure to reflect what pty's actually do. The consequence is that these settings will not stick, and cause the memcmp check to fail. An example where this affects libdivecomputer, are the two backends that require odd or even parity (e.g. vyper and iconhd). Here, the kernel will clear the PARENB flag, and thus cause the memcmp check to fail. Since this check appears to causes more trouble than it solves, let's just remove it completely! --- src/serial_posix.c | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/src/serial_posix.c b/src/serial_posix.c index 52e9191..aebcdc5 100644 --- a/src/serial_posix.c +++ b/src/serial_posix.c @@ -417,22 +417,6 @@ serial_configure (serial_t *device, int baudrate, int databits, int parity, int return -1; } - // tcsetattr() returns success if any of the requested changes could be - // successfully carried out. Therefore, when making multiple changes - // it may be necessary to follow this call with a further call to - // tcgetattr() to check that all changes have been performed successfully. - - struct termios active; - memset (&active, 0, sizeof (active)); - if (tcgetattr (device->fd, &active) != 0) { - SYSERROR (device->context, errno); - return -1; - } - if (memcmp (&tty, &active, sizeof (struct termios)) != 0) { - ERROR (device->context, "Failed to set the terminal attributes."); - return -1; - } - // Configure a custom baudrate if necessary. if (custom) { #if defined(TIOCGSERIAL) && defined(TIOCSSERIAL) && !defined(__ANDROID__)