bug#49524: 28.0.50; make-serial-process is not portable

[Ken Brown <kbrown@cornell.edu>]

[-- Attachment #1: Type: text/plain, Size: 1243 bytes --]

On 7/11/2021 12:24 PM, Eli Zaretskii wrote:
>> From: Ken Brown <kbrown@cornell.edu>
>> Date: Sun, 11 Jul 2021 11:24:58 -0400
>>
>> Fmake_serial_process calls Fserial_process_configure, which calls
>> serial_configure, which calls cfsetspeed with the speed argument equal to the
>> numerical baud rate (e.g., 9600).  But the documentation of cfsetspeed says that
>> the speed argument must be one of the Bnnn constants defined in termios.h (e.g.,
>> B9600).  See, for example,
>>
>>     https://man7.org/linux/man-pages/man3/termios.3.html
>>
>> This incorrect call of cfsetspeed happens to succeed on GNU/Linux because
>> glibc's cfsetspeed allows the argument to be the numerical baud rate, which it
>> converts to the appropriate Bnnn constant.  But I don't think emacs should be
>> relying on this undocumented behavior.  In particular, this doesn't work on
>> Cygwin.  And it wouldn't even work on GNU/Linux if emacs used the cfsetspeed
>> replacement defined in sysdep.c instead of glibc's cfsetspeed.
>>
>> I think the way to fix this is to imitate the glibc code that converts the baud
>> rate to a Bnnn constant, but maybe someone has a better idea.
> 
> Converting in sysdep.c:serial_configure sounds TRT to me.

Patch attached.

Ken

[-- Attachment #2: 0001-Fix-portability-issue-with-make-serial-process.patch --]
[-- Type: text/plain, Size: 3559 bytes --]

From aeda8d9e4432e5fac627341968e84c98f4df70bd Mon Sep 17 00:00:00 2001
From: Ken Brown <kbrown@cornell.edu>
Date: Mon, 12 Jul 2021 09:24:12 -0400
Subject: [PATCH] Fix portability issue with make-serial-process

* src/sysdep.c (struct speed_struct): New struct.
(speeds): New static array of struct speed_struct.
(convert_speed): New static function to convert a numerical baud
rate (e.g., 9600) to a Bnnn constant defined in termios.h (e.g.,
B9600).
(serial_configure): Use convert_speed to avoid cfsetspeed errors
on non-glibc platforms.  (Bug#49524)
---
 src/sysdep.c | 134 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 133 insertions(+), 1 deletion(-)

diff --git a/src/sysdep.c b/src/sysdep.c
index b8ec22d9dd..8eaee22498 100644
--- a/src/sysdep.c
+++ b/src/sysdep.c
@@ -2744,6 +2744,138 @@ cfsetspeed (struct termios *termios_p, speed_t vitesse)
 }
 #endif
 
+/* The following is based on the glibc implementation of cfsetspeed.  */
+
+struct speed_struct
+{
+  speed_t value;
+  speed_t internal;
+};
+
+static const struct speed_struct speeds[] =
+  {
+#ifdef B0
+    { 0, B0 },
+#endif
+#ifdef B50
+    { 50, B50 },
+#endif
+#ifdef B75
+    { 75, B75 },
+#endif
+#ifdef B110
+    { 110, B110 },
+#endif
+#ifdef B134
+    { 134, B134 },
+#endif
+#ifdef B150
+    { 150, B150 },
+#endif
+#ifdef B200
+    { 200, B200 },
+#endif
+#ifdef B300
+    { 300, B300 },
+#endif
+#ifdef B600
+    { 600, B600 },
+#endif
+#ifdef B1200
+    { 1200, B1200 },
+#endif
+#ifdef B1200
+    { 1200, B1200 },
+#endif
+#ifdef B1800
+    { 1800, B1800 },
+#endif
+#ifdef B2400
+    { 2400, B2400 },
+#endif
+#ifdef B4800
+    { 4800, B4800 },
+#endif
+#ifdef B9600
+    { 9600, B9600 },
+#endif
+#ifdef B19200
+    { 19200, B19200 },
+#endif
+#ifdef B38400
+    { 38400, B38400 },
+#endif
+#ifdef B57600
+    { 57600, B57600 },
+#endif
+#ifdef B76800
+    { 76800, B76800 },
+#endif
+#ifdef B115200
+    { 115200, B115200 },
+#endif
+#ifdef B153600
+    { 153600, B153600 },
+#endif
+#ifdef B230400
+    { 230400, B230400 },
+#endif
+#ifdef B307200
+    { 307200, B307200 },
+#endif
+#ifdef B460800
+    { 460800, B460800 },
+#endif
+#ifdef B500000
+    { 500000, B500000 },
+#endif
+#ifdef B576000
+    { 576000, B576000 },
+#endif
+#ifdef B921600
+    { 921600, B921600 },
+#endif
+#ifdef B1000000
+    { 1000000, B1000000 },
+#endif
+#ifdef B1152000
+    { 1152000, B1152000 },
+#endif
+#ifdef B1500000
+    { 1500000, B1500000 },
+#endif
+#ifdef B2000000
+    { 2000000, B2000000 },
+#endif
+#ifdef B2500000
+    { 2500000, B2500000 },
+#endif
+#ifdef B3000000
+    { 3000000, B3000000 },
+#endif
+#ifdef B3500000
+    { 3500000, B3500000 },
+#endif
+#ifdef B4000000
+    { 4000000, B4000000 },
+#endif
+  };
+
+/* Convert a numerical speed (e.g., 9600) to a Bnnn constant (e.g.,
+   B9600); see bug#49524.  */
+static speed_t
+convert_speed (speed_t speed)
+{
+  for (size_t i = 0; i < sizeof speeds / sizeof speeds[0]; i++)
+    {
+      if (speed == speeds[i].internal)
+	return speed;
+      else if (speed == speeds[i].value)
+	return speeds[i].internal;
+    }
+  return speed;
+}
+
 /* For serial-process-configure  */
 void
 serial_configure (struct Lisp_Process *p,
@@ -2775,7 +2907,7 @@ serial_configure (struct Lisp_Process *p,
   else
     tem = Fplist_get (p->childp, QCspeed);
   CHECK_FIXNUM (tem);
-  err = cfsetspeed (&attr, XFIXNUM (tem));
+  err = cfsetspeed (&attr, convert_speed (XFIXNUM (tem)));
   if (err != 0)
     report_file_error ("Failed cfsetspeed", tem);
   childp2 = Fplist_put (childp2, QCspeed, tem);
-- 
2.32.0