GNU bug report logs -
#49524
28.0.50; make-serial-process is not portable
Previous Next
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.
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):
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: 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):
[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):
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: 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):
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.