Debian Bug report logs - #45796
hurd: term doesn't recognize open failures

Package: hurd; Maintainer for hurd is GNU Hurd Maintainers <debian-hurd@lists.debian.org>; Source for hurd is src:hurd.

Reported by: Marcus.Brinkmann@ruhr-uni-bochum.de

Date: Wed, 22 Sep 1999 21:03:09 UTC

Severity: normal

Tags: upstream

Forwarded to https://savannah.gnu.org/bugs/index.php?func=detailitem&item_id=15319

Reply or subscribe to this bug.

Toggle useless messages

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


Report forwarded to debian-bugs-dist@lists.debian.org, GNU Hurd Maintainers <bug-hurd@gnu.org>:
Bug#45796; Package hurd. Full text and rfc822 format available.

Acknowledgement sent to Marcus.Brinkmann@ruhr-uni-bochum.de:
New Bug report received and forwarded. Copy sent to GNU Hurd Maintainers <bug-hurd@gnu.org>. Full text and rfc822 format available.

Message #5 received at submit@bugs.debian.org (full text, mbox):

From: Marcus.Brinkmann@ruhr-uni-bochum.de
To: submit@bugs.debian.org
Subject: hurd: term doesn't recognize open failures
Date: Wed, 22 Sep 1999 22:31:34 +0200
Package: hurd
Version: N/A
Severity: normal

Hello,

term doesn't barf at you if a device doesn't exist.
The script below shows that storio does.

Script started on Wed Sep 22 22:02:55 1999
hurd:/dev# devprobe com0 hd2s5 hd1s2
hd2s5
hurd:/dev# showtrans com0 hd2s5 hd1s2
com0: /hurd/term /dev/com0 device com0
hd2s5: /hurd/storeio hd2s5
hd1s2: /hurd/storeio hd1s2
hurd:/dev# cat com0
^C
hurd:/dev# cat hd1s2
cat: hd1s2: Device not configured
hurd:/dev# cat hd2s5
<lots of garbage here of course>

The problem is that with device_open_request, it seems to be hard
(impossible?) to return an error to the user. This is suboptimal. The same
problem exists in the streamdev translator, btw. I can transfer any fix from
term to streamdev (or at least hope to be able to do so).

BTW, what is the advantage of device_open_reply as opposed to device_open?
Is it a reqirement for the other _reply functions? I can't see a reason why
opening a device should block or something like this...

Thanks,
Marcus

-- System Information
Debian Release: potato
Kernel Version: Linux ulysses 2.2.12 #1 Mit Sep 15 03:28:57 CEST 1999 i586 unknown



Information forwarded to debian-bugs-dist@lists.debian.org, GNU Hurd Maintainers <bug-hurd@gnu.org>:
Bug#45796; Package hurd. Full text and rfc822 format available.

Acknowledgement sent to tb@MIT.EDU (Thomas Bushnell, BSG):
Extra info received and forwarded to list. Copy sent to GNU Hurd Maintainers <bug-hurd@gnu.org>. Full text and rfc822 format available.

Message #10 received at 45796@bugs.debian.org (full text, mbox):

From: tb@MIT.EDU (Thomas Bushnell, BSG)
To: Marcus.Brinkmann@ruhr-uni-bochum.de
Cc: 45796@bugs.debian.org, bugs-hurd@gnu.org
Subject: Re: Bug#45796: hurd: term doesn't recognize open failures
Date: 24 Sep 1999 02:18:02 -0400
Marcus.Brinkmann@ruhr-uni-bochum.de writes:

> BTW, what is the advantage of device_open_reply as opposed to device_open?
> Is it a reqirement for the other _reply functions? I can't see a reason why
> opening a device should block or something like this...

Unfortunately, the device open blocks until carrier is established
(which is the Unix semantics of open).  So a fix to use device_open
will result in a block at the wrong time.  (Note that devio.c is
careful to interpret the reply as "carrier has turned on" at the end
of the function:

  report_carrier_on ();
  if (err)
    devio_desert_dtr ();

  open_pending = NOTPENDING;

The Mach device interface has many problems, especially for terminals,
where the wrong level of control is being provided.  

However, streamdev needs none of this complexity.  I haven't had a
chance to look at your code yet, but I suspect the entire level of
indirection between users.c and the "bottom halves" is not necessary,
and there is certainly no need to worry about the weird carrier
semantics of terminals.  Just go ahead and have the open_hook do a
simple device_open and report the error appropriately.  

The problem here with term is a real bug tho, so I'll work on it.  But
it shouldn't delay you from making streamdev do the right thing.

(Note that I was aware of this problem when I wrote term; the relevant
code in devio.c says "Bogus" and then tries to recover somehow.)  I'll
probably make a fix by adding a flag and a global variable to
communicate the error back, but this should not be copied into
streamdev, which doesn't need to be so complex anyhow.

Thomas


Information forwarded to debian-bugs-dist@lists.debian.org, GNU Hurd Maintainers <bug-hurd@gnu.org>:
Bug#45796; Package hurd. Full text and rfc822 format available.

Acknowledgement sent to tb@MIT.EDU (Thomas Bushnell, BSG):
Extra info received and forwarded to list. Copy sent to GNU Hurd Maintainers <bug-hurd@gnu.org>. Full text and rfc822 format available.

Information forwarded to debian-bugs-dist@lists.debian.org, GNU Hurd Maintainers <bug-hurd@gnu.org>:
Bug#45796; Package hurd. Full text and rfc822 format available.

Acknowledgement sent to Roland McGrath <roland@gnu.org>:
Extra info received and forwarded to list. Copy sent to GNU Hurd Maintainers <bug-hurd@gnu.org>. Full text and rfc822 format available.

Message #20 received at 45796@bugs.debian.org (full text, mbox):

From: Roland McGrath <roland@gnu.org>
To: Marcus.Brinkmann@ruhr-uni-bochum.de, 45796@bugs.debian.org
Subject: Re: Bug#45796: hurd: term doesn't recognize open failures
Date: Fri, 1 Oct 1999 17:46:18 -0400
I've checked in the following changes, which seem to work for me.
That is, bogus devices return an error and /dev/console still works.
If things are good for you with this, please close the bug.
Thomas might well still want to rewrite this, but this seems to do for now.


Index: ChangeLog
===================================================================
RCS file: /afs/sipb.mit.edu/project/hurddev/cvsroot/hurd/term/ChangeLog,v
retrieving revision 1.55
diff -u -b -r1.55 ChangeLog
--- ChangeLog	1999/09/13 06:34:29	1.55
+++ ChangeLog	1999/10/01 21:37:48
@@ -1,3 +1,16 @@
+1999-10-01  Roland McGrath  <roland@baalperazim.frob.com>
+
+	* term.h (NO_DEVICE): New macro, bit for termflags.
+	(termflags): Change type to uint_fast32_t.
+	* devio.c (device_open_reply): For D_NO_SUCH_DEVICE error reply, set
+	NO_DEVICE flag in termflags.
+	* users.c (open_hook): If NO_DEVICE flag set, return ENXIO immediately.
+	If we put out an open request, check for that bit as well as
+	NO_CARRIER changing in termflags and diagnose with ENXIO.
+	* Makefile (device_replyServer-CPPFLAGS): New variable, turn off
+	TypeCheck for this stub.  This is necessary for error replies to get
+	through to our server-side functions in devio.c.
+
 1999-09-13  Roland McGrath  <roland@baalperazim.frob.com>
 
 	* users.c: Reverted changes related to io_map_segment.
Index: Makefile
===================================================================
RCS file: /afs/sipb.mit.edu/project/hurddev/cvsroot/hurd/term/Makefile,v
retrieving revision 1.6
diff -u -b -r1.6 Makefile
--- Makefile	1997/02/20 02:40:15	1.6
+++ Makefile	1999/10/01 21:19:01
@@ -1,5 +1,5 @@
 # 
-#   Copyright (C) 1995, 1996, 1997 Free Software Foundation, Inc.
+#   Copyright (C) 1995, 1996, 1997, 1999 Free Software Foundation, Inc.
 #   Written by Michael I. Bushnell, p/BSG.
 #
 #   This file is part of the GNU Hurd.
@@ -30,3 +30,5 @@
 OBJS = $(subst .c,.o,$(SRCS)) termServer.o device_replyServer.o tioctlServer.o ourmsgUser.o 
 
 include ../Makeconf
+
+device_replyServer-CPPFLAGS = -DTypeCheck=0 -Wno-unused # XXX
Index: devio.c
===================================================================
RCS file: /afs/sipb.mit.edu/project/hurddev/cvsroot/hurd/term/devio.c,v
retrieving revision 1.23
diff -u -b -r1.23 devio.c
--- devio.c	1999/05/30 01:48:12	1.23
+++ devio.c	1999/10/01 21:35:04
@@ -506,6 +506,13 @@
 
       if (returncode != 0)
 	{
+	  /* Note that DEVICE is total garbage (not a real port name at all!)
+	     in this case.  */
+
+	  if (returncode == D_NO_SUCH_DEVICE)
+	    /* Record that the device does not exist.  */
+	    termflags |= NO_DEVICE;
+
 	  /* Bogus. */
 	  report_carrier_on ();
 	  report_carrier_off ();
Index: term.h
===================================================================
RCS file: /afs/sipb.mit.edu/project/hurddev/cvsroot/hurd/term/term.h,v
retrieving revision 1.42
diff -u -b -r1.42 term.h
--- term.h	1999/07/24 00:22:03	1.42
+++ term.h	1999/10/01 21:32:45
@@ -25,6 +25,7 @@
 #include <sys/types.h>
 #include <sys/mman.h>
 #include <fcntl.h>
+#include <stdint.h>
 
 #undef MDMBUF
 #undef ECHO
@@ -65,7 +66,7 @@
 struct termios termstate;
 
 /* Other state with the following bits: */
-long termflags;
+uint_fast32_t termflags;
 
 #define USER_OUTPUT_SUSP  0x00000001 /* user has suspended output */
 #define TTY_OPEN	  0x00000002 /* someone has us open */
@@ -78,6 +79,7 @@
 #define EXCL_USE          0x00000100 /* user accessible exclusive use */
 #define NO_OWNER          0x00000200 /* there is no foreground_id */
 #define ICKY_ASYNC	  0x00000400 /* some user has set O_ASYNC */
+#define	NO_DEVICE	  0x00000800 /* the device does not exist */
 
 #define QUEUE_LOWAT 100
 #define QUEUE_HIWAT 300
Index: users.c
===================================================================
RCS file: /afs/sipb.mit.edu/project/hurddev/cvsroot/hurd/term/users.c,v
retrieving revision 1.85
diff -u -b -r1.85 users.c
--- users.c	1999/09/13 06:34:33	1.85
+++ users.c	1999/10/01 21:35:42
@@ -144,10 +144,19 @@
     return pty_open_hook (cntl, user, flags);
 
   if ((flags & (O_READ|O_WRITE)) == 0)
+    /* Not asking for a port that can do i/o (just stat or chmod or whatnot),
+       so there is nothing else we need to think about.  */
     return 0;
 
   mutex_lock (&global_lock);
 
+  if (termflags & NO_DEVICE)
+    {
+      /* We previously discovered that the underlying device doesn't exist.  */
+      mutex_unlock (&global_lock);
+      return ENXIO;
+    }
+
   if (!(termflags & TTY_OPEN))
     {
       bzero (&termstate, sizeof termstate);
@@ -194,10 +203,19 @@
     }
 
   /* Wait for carrier to turn on. */
-  while (((termflags & NO_CARRIER) && !(termstate.c_cflag & CLOCAL))
+  while ((termflags & (NO_CARRIER|NO_DEVICE)) == NO_CARRIER
+	 && !(termstate.c_cflag & CLOCAL)
 	 && !(flags & O_NONBLOCK)
 	 && !cancel)
     cancel = hurd_condition_wait (&carrier_alert, &global_lock);
+
+  if (termflags & NO_DEVICE)
+    {
+      /* The open of the underlying device returned an error indicating
+	 that no such device exists.  */
+      mutex_unlock (&global_lock);
+      return ENXIO;
+    }
 
   if (cancel)
     {


Tags added: upstream Request was from Samuel Thibault <samuel.thibault@ens-lyon.org> to control@bugs.debian.org. Full text and rfc822 format available.

Noted your statement that Bug has been forwarded to https://savannah.gnu.org/bugs/index.php?func=detailitem&item_id=15319. Request was from Samuel Thibault <samuel.thibault@ens-lyon.org> to control@bugs.debian.org. Full text and rfc822 format available.

Send a report that this bug log contains spam.


Debian bug tracking system administrator <owner@bugs.debian.org>. Last modified: Mon Apr 21 12:12:18 2014; Machine Name: beach.debian.org

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