NetBSD Problem Report #48720

From www@NetBSD.org  Sun Apr  6 17:04:12 2014
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.netbsd.org [149.20.53.66])
	(using TLSv1 with cipher ECDHE-RSA-AES256-SHA (256/256 bits))
	(Client CN "mail.netbsd.org", Issuer "Postmaster NetBSD.org" (verified OK))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 1F1EDA5805
	for <gnats-bugs@gnats.NetBSD.org>; Sun,  6 Apr 2014 17:04:12 +0000 (UTC)
Message-Id: <20140406170410.97A78A580E@mollari.NetBSD.org>
Date: Sun,  6 Apr 2014 17:04:10 +0000 (UTC)
From: src153@gmail.com
Reply-To: src153@gmail.com
To: gnats-bugs@NetBSD.org
Subject: POSIX tcsetattr() fails B0
X-Send-Pr-Version: www-1.0

>Number:         48720
>Category:       bin
>Synopsis:       POSIX tcsetattr() fails B0
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    bin-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Apr 06 17:05:00 +0000 2014
>Closed-Date:    Tue Apr 08 06:44:36 +0000 2014
>Last-Modified:  Tue Apr 08 12:55:00 +0000 2014
>Originator:     John Kelly
>Release:        6.1.3
>Organization:
none
>Environment:
NetBSD ... 6.1.3 NetBSD 6.1.3 (GENERIC) i386

>Description:
POSIX says:

If the output baud rate stored in the termios structure pointed to by termios_p is the zero baud rate, B0, the modem control lines shall no longer be asserted. Normally, this shall disconnect the line.

http://pubs.opengroup.org/onlinepubs/9699919799/functions/tcsetattr.html

NetBSD tcsetattr() rejects B0 as invalid argument.

>How-To-Repeat:
# include <errno.h>
# include <fcntl.h>
# include <termios.h>
# include <unistd.h>
# include <stdio.h>
# include <string.h>

int fd, rv;
struct termios term;

int
main (void)
{

    fd = open ("/dev/tty00", O_RDWR | O_NOCTTY | O_NONBLOCK);
    if (fd == -1) {
        printf ("failure opening tty: %s\n", strerror (errno));
        return 1;
    }

    rv = tcgetattr (fd, &term);
    if (rv == -1) {
        printf ("failure getting tty termios: %s\n", strerror (errno));
        return 1;
    }

    rv = cfsetspeed (&term, B0);
    if (rv == -1) {
        printf ("failure setting tty baud: %s\n", strerror (errno));
        return 1;
    }

    rv = tcsetattr (fd, TCSANOW, &term);
    if (rv == -1) {
        printf ("failure setting tty termios: %s\n", strerror (errno));
        return 1;
    }

    return 0;
}

>Fix:
I browsed some of the source, but didn't see the problem. Looks like the fix could get messy though. DTR must stay down as long as the baud rate remains 0.

POSIX does not specify that RTS should be down too, and there are good reasons for leaving it untouched. DTR down, is all you need to disconnect the line.

>Release-Note:

>Audit-Trail:
From: "Christos Zoulas" <christos@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/48720 CVS commit: src/sys/dev/ic
Date: Sun, 6 Apr 2014 19:29:58 -0400

 Module Name:	src
 Committed By:	christos
 Date:		Sun Apr  6 23:29:58 UTC 2014

 Modified Files:
 	src/sys/dev/ic: com.c

 Log Message:
 PR/48720: John Kelly: com driver does not allow B0 to be set. The code to
 handle B0 was commented out for no reason given at revision 1.99. POSIX
 mandates to hangup on B0:
 http://pubs.opengroup.org/onlinepubs/9699919799/functions/tcsetattr.html


 To generate a diff of this commit:
 cvs rdiff -u -r1.323 -r1.324 src/sys/dev/ic/com.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: John Kelly <src153@gmail.com>
To: gnats-bugs@netbsd.org
Cc: Christos Zoulas <christos@netbsd.org>
Subject: Re: PR/48720 CVS commit: src/sys/dev/ic
Date: Mon, 7 Apr 2014 22:02:37 +0000

 On 4/6/14, Christos Zoulas <christos@netbsd.org> wrote:

 >  Log Message:
 >  PR/48720: John Kelly: com driver does not allow B0 to be set. The code to
 >  handle B0 was commented out for no reason given at revision 1.99. POSIX
 >  mandates to hangup on B0:
 >  http://pubs.opengroup.org/onlinepubs/9699919799/functions/tcsetattr.html

 >  To generate a diff of this commit:
 >  cvs rdiff -u -r1.323 -r1.324 src/sys/dev/ic/com.c

 Thanks for a quick response. Your patch allows B0 to be set, which
 drops DTR. But once that's done, changing the baud to a non-zero value
 fails to bring DTR back up.


 @@ -1426,9 +1424,9 @@
  <----->}
  <----->sc->sc_msr_mask = sc->sc_msr_cts | sc->sc_msr_dcd;
 .
 -#if 0
  <----->if (ospeed == 0)
  <-----><------>CLR(sc->sc_mcr, sc->sc_mcr_dtr);
 +#if 0
  <----->else
  <-----><------>SET(sc->sc_mcr, sc->sc_mcr_dtr);
  #endif


 SET is still commented out, but it's needed too. Reactivating the else
 and SET allows a non zero baud rate to bring DTR back up. That fixes
 the POSIX compliance problem.

 But the fix may be risky. There are things happening with DTR in com.c
 that I don't fully understand yet. Looking at the CVS history, I see
 Hannum made some comments about DTR. Maybe he was trying to fix some
 other problem, when he caused the POSIX problem.

 At least I know where the problem is now. I can test it further when I
 have more time. If you want to revert the fix for safety's sake, it's
 OK with me.

 I leave it to your judgement.

From: christos@zoulas.com (Christos Zoulas)
To: gnats-bugs@NetBSD.org, gnats-admin@netbsd.org, netbsd-bugs@netbsd.org, 
	src153@gmail.com
Cc: 
Subject: Re: PR/48720 CVS commit: src/sys/dev/ic
Date: Mon, 7 Apr 2014 20:09:33 -0400

 On Apr 7, 10:05pm, src153@gmail.com (John Kelly) wrote:
 -- Subject: Re: PR/48720 CVS commit: src/sys/dev/ic

 | The following reply was made to PR bin/48720; it has been noted by GNATS.
 | 
 | From: John Kelly <src153@gmail.com>
 | To: gnats-bugs@netbsd.org
 | Cc: Christos Zoulas <christos@netbsd.org>
 | Subject: Re: PR/48720 CVS commit: src/sys/dev/ic
 | Date: Mon, 7 Apr 2014 22:02:37 +0000
 | 
 |  On 4/6/14, Christos Zoulas <christos@netbsd.org> wrote:
 |  
 |  >  Log Message:
 |  >  PR/48720: John Kelly: com driver does not allow B0 to be set. The code to
 |  >  handle B0 was commented out for no reason given at revision 1.99. POSIX
 |  >  mandates to hangup on B0:
 |  >  http://pubs.opengroup.org/onlinepubs/9699919799/functions/tcsetattr.html
 |  
 |  >  To generate a diff of this commit:
 |  >  cvs rdiff -u -r1.323 -r1.324 src/sys/dev/ic/com.c
 |  
 |  Thanks for a quick response. Your patch allows B0 to be set, which
 |  drops DTR. But once that's done, changing the baud to a non-zero value
 |  fails to bring DTR back up.
 |  
 |  
 |  @@ -1426,9 +1424,9 @@
 |   <----->}
 |   <----->sc->sc_msr_mask = sc->sc_msr_cts | sc->sc_msr_dcd;
 |  .
 |  -#if 0
 |   <----->if (ospeed == 0)
 |   <-----><------>CLR(sc->sc_mcr, sc->sc_mcr_dtr);
 |  +#if 0
 |   <----->else
 |   <-----><------>SET(sc->sc_mcr, sc->sc_mcr_dtr);
 |   #endif
 |  
 |  
 |  SET is still commented out, but it's needed too. Reactivating the else
 |  and SET allows a non zero baud rate to bring DTR back up. That fixes
 |  the POSIX compliance problem.
 |  
 |  But the fix may be risky. There are things happening with DTR in com.c
 |  that I don't fully understand yet. Looking at the CVS history, I see
 |  Hannum made some comments about DTR. Maybe he was trying to fix some
 |  other problem, when he caused the POSIX problem.
 |  
 |  At least I know where the problem is now. I can test it further when I
 |  have more time. If you want to revert the fix for safety's sake, it's
 |  OK with me.
 |  
 |  I leave it to your judgement.

 I wanted to be safe by not dealing with resetting dtr... hoping that something
 else would do it, but I've committed a better patch.

 christos

From: John Kelly <src153@gmail.com>
To: Christos Zoulas <christos@zoulas.com>
Cc: gnats-bugs@netbsd.org, gnats-admin@netbsd.org, netbsd-bugs@netbsd.org
Subject: Re: PR/48720 CVS commit: src/sys/dev/ic
Date: Tue, 8 Apr 2014 06:05:25 +0000

 --001a11c2c338275c5704f681c578
 Content-Type: text/plain; charset=ISO-8859-1

 On Tue, Apr 8, 2014 at 12:09 AM, Christos Zoulas <christos@zoulas.com>wrote:

 I've committed a better patch


 Looks good. Works too. Thanks for the fix.

 --001a11c2c338275c5704f681c578
 Content-Type: text/html; charset=ISO-8859-1
 Content-Transfer-Encoding: quoted-printable

 <div dir=3D"ltr"><div class=3D"gmail_extra"><div class=3D"gmail_quote">On T=
 ue, Apr 8, 2014 at 12:09 AM, Christos <span style class=3D"">Zoulas</span> =
 <span dir=3D"ltr">&lt;<a href=3D"mailto:christos@zoulas.com" target=3D"_bla=
 nk"><span style class=3D"">christos</span>@<span style class=3D"">zoulas</s=
 pan>.com</a>&gt;</span> wrote:<br>
 <div></div><br><blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex=
 ;border-left:1px #ccc solid;padding-left:1ex">I&#39;ve committed a better p=
 atch</blockquote><div><br></div><div>Looks good. Works too. Thanks for the =
 fix.<br>
 <br></div></div></div></div>

 --001a11c2c338275c5704f681c578--

State-Changed-From-To: open->closed
State-Changed-By: wiz@NetBSD.org
State-Changed-When: Tue, 08 Apr 2014 06:44:36 +0000
State-Changed-Why:
Confirmed fixed, thanks for the feedback!


From: christos@zoulas.com (Christos Zoulas)
To: John Kelly <src153@gmail.com>
Cc: gnats-bugs@netbsd.org, gnats-admin@netbsd.org, netbsd-bugs@netbsd.org
Subject: Re: PR/48720 CVS commit: src/sys/dev/ic
Date: Tue, 8 Apr 2014 08:52:01 -0400

 On Apr 8,  6:05am, src153@gmail.com (John Kelly) wrote:
 -- Subject: Re: PR/48720 CVS commit: src/sys/dev/ic

 | Looks good. Works too. Thanks for the fix.

 Yup, the good thing about it, is that it does not affect the regular case.

 christos

>Unformatted:

Home	PR Database Search