[Nut-upsdev] [PATCH] fix this spurious tcsetattr()/tcgetattr() change once and for all

[Greg A. Woods]

From: "Greg A. Woods" <woods at planix.com>

The most likely cause of all spurious differences between what was set
on the port with tcsetattr() and what tcgetattr() shows are likely to do
with the c_local PENDIN flag, which is a status bit, not a control bit.
It will change when there's unread pending input, which can be quite
often on an APC UPS.

The right way to compare struct termios values is to clear the status
flags _after_ the tcsetattr() and of course after the tcgetattr() call
and then compare the result with what was set.

Also set NOKERNINFO, if available, as we don't want the UPS or noise on
the line to accidentally trigger status output back to the UPS, and
finally make sure IEXTEN is also cleared along with ISIG since it too
can cause weird things to happen.

This change also adds some debug code to show any differences in the
structures in a logical manner in debug output (and squashes one tiny
compiler warning).
---
 drivers/apcsmart.c |   80 ++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 77 insertions(+), 3 deletions(-)

diff --git a/drivers/apcsmart.c b/drivers/apcsmart.c
index 932dd9c..430368e 100644
--- a/drivers/apcsmart.c
+++ b/drivers/apcsmart.c
@@ -188,7 +188,10 @@ static void apc_ser_set(void)
 	tio.c_cflag |= (CS8 | CLOCAL | CREAD);
 
 	tio.c_lflag |= ICANON;
-	tio.c_lflag &= ~ISIG;
+#ifdef NOKERNINFO
+	tio.c_lflag |= NOKERNINFO;
+#endif
+	tio.c_lflag &= ~(ISIG | IEXTEN);
 
 	tio.c_iflag |= (IGNCR | IGNPAR);
 	tio.c_iflag &= ~(IXON | IXOFF);
@@ -220,12 +223,83 @@ static void apc_ser_set(void)
 	if (tcsetattr(upsfd, TCSANOW, &tio))
 		fatal_with_errno(EXIT_FAILURE, "tcsetattr(%s)", device_path);
 
+	/* clear status flags so that they don't affect our binary compare */
+#ifdef PENDIN
+	tio.c_lflag &= ~PENDIN;
+#endif
+#ifdef FLUSHO
+	tio.c_lflag &= ~FLUSHO;
+#endif
+
 	memset(&tio_chk, 0, sizeof(tio_chk));
 	if (tcgetattr(upsfd, &tio_chk))
 		fatal_with_errno(EXIT_FAILURE, "tcgetattr(%s)", device_path);
-	if (memcmp(&tio_chk, &tio, sizeof(tio)))
+
+	/* clear status flags so that they don't affect our binary compare */
+#ifdef PENDIN
+	tio_chk.c_lflag &= ~PENDIN;
+#endif
+#ifdef FLUSHO
+	tio_chk.c_lflag &= ~FLUSHO;
+#endif
+
+	if (memcmp(&tio_chk, &tio, sizeof(tio))) {
+		struct cchar {
+			const char *name;
+			int sub;
+			u_char def;
+		};
+		const struct cchar cchars1[] = {
+			{ "discard",	VDISCARD, 	CDISCARD },
+			{ "dsusp", 	VDSUSP,		CDSUSP },
+			{ "eof",	VEOF,		CEOF },
+			{ "eol",	VEOL,		CEOL },
+			{ "eol2",	VEOL2,		CEOL },
+			{ "erase",	VERASE,		CERASE },
+			{ "intr",	VINTR,		CINTR },
+			{ "kill",	VKILL,		CKILL },
+			{ "lnext",	VLNEXT,		CLNEXT },
+			{ "min",	VMIN,		CMIN },
+			{ "quit",	VQUIT,		CQUIT },
+			{ "reprint",	VREPRINT, 	CREPRINT },
+			{ "start",	VSTART,		CSTART },
+			{ "status",	VSTATUS, 	CSTATUS },
+			{ "stop",	VSTOP,		CSTOP },
+			{ "susp",	VSUSP,		CSUSP },
+			{ "time",	VTIME,		CTIME },
+			{ "werase",	VWERASE,	CWERASE },
+			{ .name = NULL },
+		};
+		const struct cchar *cp;
+		struct termios *tp;
+
 		upslogx(LOG_NOTICE, "%s: device reports different attributes than what were set", device_path);
 
+		/*
+		 * According to the manual the most common problem is
+		 * mis-matched combinations of input and output baud rates.  If
+		 * the combinaion is not supported then neither are changed.
+		 * This should not be a problem here since we set them both to
+		 * the same extremely common rate of 2400.
+		 */
+
+		tp = &tio;
+		upsdebugx(1, "tcsetattr(): gfmt1:cflag=%x:iflag=%x:lflag=%x:oflag=%x:",
+			  (unsigned int) tp->c_cflag, (unsigned int) tp->c_iflag,
+			  (unsigned int) tp->c_lflag, (unsigned int) tp->c_oflag);
+		for (cp = cchars1; cp->name; ++cp)
+			upsdebugx(1, "\t%s=%x:", cp->name, tp->c_cc[cp->sub]);
+		upsdebugx(1, "\tispeed=%d:ospeed=%d", (int) cfgetispeed(tp), (int) cfgetospeed(tp));
+
+		tp = &tio_chk;
+		upsdebugx(1, "tcgetattr(): gfmt1:cflag=%x:iflag=%x:lflag=%x:oflag=%x:",
+			  (unsigned int) tp->c_cflag, (unsigned int) tp->c_iflag,
+			  (unsigned int) tp->c_lflag, (unsigned int) tp->c_oflag);
+		for (cp = cchars1; cp->name; ++cp)
+			upsdebugx(1, "\t%s=%x:", cp->name, tp->c_cc[cp->sub]);
+		upsdebugx(1, "\tispeed=%d:ospeed=%d", (int) cfgetispeed(tp), (int) cfgetospeed(tp));
+	}
+
 	cable = getval("cable");
 	if (cable && !strcasecmp(cable, ALT_CABLE_1)) {
 		if (ser_set_dtr(upsfd, 1) == -1)
@@ -265,7 +339,7 @@ static void ups_status_set(void)
 
 static void check_temperature(void)
 {
-	char *val;
+	const char *val;
 	int maxtemp = APC_MELTING_TEMP;
 
 	if ((val = getval("maxtemp"))) {
-- 
1.7.9.2