GNU bug report logs - #49524
28.0.50; make-serial-process is not portable

Previous Next

Package: emacs;

Reported by: Ken Brown <kbrown <at> cornell.edu>

Date: Sun, 11 Jul 2021 15:31:01 UTC

Severity: normal

Found in version 28.0.50

Done: Ken Brown <kbrown <at> cornell.edu>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 49524 in the body.
You can then email your comments to 49524 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#49524; Package emacs. (Sun, 11 Jul 2021 15:31:01 GMT) Full text and rfc822 format available.

Acknowledgement sent to Ken Brown <kbrown <at> cornell.edu>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Sun, 11 Jul 2021 15:31:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Ken Brown <kbrown <at> cornell.edu>
To: bug-gnu-emacs <at> gnu.org
Subject: 28.0.50; make-serial-process is not portable
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.

By the way, I came across this issue while investigating the failure of 
process-tests/fd-setsize-no-crash/make-serial-process on Cygwin.







Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49524; Package emacs. (Sun, 11 Jul 2021 16:26:02 GMT) Full text and rfc822 format available.

Message #8 received at 49524 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Ken Brown <kbrown <at> cornell.edu>
Cc: 49524 <at> debbugs.gnu.org
Subject: Re: bug#49524: 28.0.50; make-serial-process is not portable
Date: Sun, 11 Jul 2021 19:24:50 +0300
> From: Ken Brown <kbrown <at> 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.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49524; Package emacs. (Mon, 12 Jul 2021 13:29:01 GMT) Full text and rfc822 format available.

Message #11 received at 49524 <at> debbugs.gnu.org (full text, mbox):

From: Ken Brown <kbrown <at> cornell.edu>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 49524 <at> debbugs.gnu.org
Subject: Re: bug#49524: 28.0.50; make-serial-process is not portable
Date: Mon, 12 Jul 2021 09:27:44 -0400
[Message part 1 (text/plain, inline)]
On 7/11/2021 12:24 PM, Eli Zaretskii wrote:
>> From: Ken Brown <kbrown <at> 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
[0001-Fix-portability-issue-with-make-serial-process.patch (text/plain, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49524; Package emacs. (Mon, 12 Jul 2021 21:36:01 GMT) Full text and rfc822 format available.

Message #14 received at 49524 <at> debbugs.gnu.org (full text, mbox):

From: Ken Brown <kbrown <at> cornell.edu>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 49524 <at> debbugs.gnu.org
Subject: Re: bug#49524: 28.0.50; make-serial-process is not portable
Date: Mon, 12 Jul 2021 17:35:40 -0400
On 7/12/2021 9:27 AM, Ken Brown wrote:
> On 7/11/2021 12:24 PM, Eli Zaretskii wrote:
>>> From: Ken Brown <kbrown <at> 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.

BTW, we've decided to change Cygwin's cfsetspeed to be compatible with glibc's, 
so the problem fixed by my patch won't exist on Cygwin going forward.  And I 
checked FreeBSD out of curiosity and found that there's no issue there either 
because of the way they define the Bnnn constants:

#define	B0	0
#define	B50	50
#define	B75	75
...

So I should probably remove the reference to non-glibc platforms in my commit 
message.

Ken




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49524; Package emacs. (Tue, 13 Jul 2021 11:20:02 GMT) Full text and rfc822 format available.

Message #17 received at 49524 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Ken Brown <kbrown <at> cornell.edu>
Cc: 49524 <at> debbugs.gnu.org
Subject: Re: bug#49524: 28.0.50; make-serial-process is not portable
Date: Tue, 13 Jul 2021 14:19:11 +0300
> From: Ken Brown <kbrown <at> cornell.edu>
> Cc: 49524 <at> debbugs.gnu.org
> Date: Mon, 12 Jul 2021 17:35:40 -0400
> 
> So I should probably remove the reference to non-glibc platforms in my commit 
> message.

Yes, something to the effect of compliance with advertised APIs would
probably be better.

Thanks.




Reply sent to Ken Brown <kbrown <at> cornell.edu>:
You have taken responsibility. (Tue, 13 Jul 2021 13:13:02 GMT) Full text and rfc822 format available.

Notification sent to Ken Brown <kbrown <at> cornell.edu>:
bug acknowledged by developer. (Tue, 13 Jul 2021 13:13:02 GMT) Full text and rfc822 format available.

Message #22 received at 49524-done <at> debbugs.gnu.org (full text, mbox):

From: Ken Brown <kbrown <at> cornell.edu>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 49524-done <at> debbugs.gnu.org
Subject: Re: bug#49524: 28.0.50; make-serial-process is not portable
Date: Tue, 13 Jul 2021 09:11:12 -0400
On 7/13/2021 7:19 AM, Eli Zaretskii wrote:
>> From: Ken Brown <kbrown <at> cornell.edu>
>> Cc: 49524 <at> debbugs.gnu.org
>> Date: Mon, 12 Jul 2021 17:35:40 -0400
>>
>> So I should probably remove the reference to non-glibc platforms in my commit
>> message.
> 
> Yes, something to the effect of compliance with advertised APIs would
> probably be better.

Done.  Closing.




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Wed, 11 Aug 2021 11:24:06 GMT) Full text and rfc822 format available.

This bug report was last modified 2 years and 230 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.