[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

more MiNT 1.09 bugs



I encountered two problems with MiNT 1.09. First a short description,
followed by context diff's to patch them. (The patches for both of them
are mixed up in the same file, sorry :-).

1.)	If you change the flow-control state of a serial device
	(u:/dev/modem1, ...) with the TIOCSFLAGS ioctl() (TANDEM, RTSCTS
	Bits), then you dont't get the correct state of them with
	following TIOCGFLAGS ioctl()'s (they rely on the saved state
	in the sg.sg_flags member of the struct tty, which is not
	updated by TIOCSFLAGS calls).

2.)	Fselect() does not work properly when several processes are  selecting
	on the same file (there're no provisions to handle collisions). For
	instance, if two processes are selecting on the same sets of files,
	the second one doing the Fselect() is blocked forever (provided no
	timeout is specified and no signals are delivered), even if the
	specified descriptors become ready for doing IO. The fix requires
	support from the device-drivers select() routine. The return values
	from select() should be:
		0 -- not ready for IO, register for selecting (not changed)
		1 -- ready for IO (not changed)
		2 -- collision (not ready, but no room to hold
		     another process for selection on that file)
	All the other work is done in f_select(). Because collisions happen
	rather rarely, it's not worth to make the device-drivers select()
	able to hold an arbitrary number of processes for selection on the same
	file or device.
	The device drivers should just be able to hold one process and return
	'2' at further requests to hold another process at the same time for
	the same file or device (see pipe_select for example).

The following cdiffs are relative to MiNT 1.09, it would be nice if they
could be applied to new MiNT versions (to Eric ...).

cheers, Kay Roemer.

*** biosfs.c.orig	Sun Sep  5 20:18:00 1993
--- biosfs.c	Mon Sep 13 21:41:28 1993
***************
*** 1144,1150 ****
--- 1144,1154 ----
  				oflags |= (ucr & 0x2) ? T_EVENP : T_ODDP;
  			}
  			if (mode == TIOCSFLAGS) {
+ 				ushort *sg_flags =
+ 					&((struct tty *)f->devinfo)->sg.sg_flags;
  				flags = (*(unsigned short *)buf);
+ 				*sg_flags &= ~(T_RTSCTS|T_TANDEM);
+ 				*sg_flags |= flags & (T_RTSCTS|T_TANDEM);
  				if (flags & T_EVENP) {
  					ucr |= 0x6;
  				} else if (flags & T_ODDP) {
***************
*** 1236,1243 ****
  		}
  		if (tty) {
  		/* avoid collisions with other processes */
! 			if (!tty->rsel)
! 				tty->rsel = p;
  		}
  		return 0;
  	} else if (mode == O_WRONLY) {
--- 1240,1248 ----
  		}
  		if (tty) {
  		/* avoid collisions with other processes */
! 			if (tty->rsel)
! 				return 2; /* collsion */
! 			tty->rsel = p;
  		}
  		return 0;
  	} else if (mode == O_WRONLY) {
***************
*** 1246,1253 ****
  			return 1;
  		}
  		if (tty) {
! 			if (!tty->wsel)
! 				tty->wsel = p;
  		}
  		return 0;
  	}
--- 1251,1259 ----
  			return 1;
  		}
  		if (tty) {
! 			if (tty->wsel)
! 				return 2; /* collision */
! 			tty->wsel = p;
  		}
  		return 0;
  	}
***************
*** 1482,1489 ****
  	if (mousetail - mousehead)
  		return 1;	/* input waiting already */
  
! 	if (!mousersel)
! 		mousersel = p;
  	return 0;
  }
  
--- 1488,1496 ----
  	if (mousetail - mousehead)
  		return 1;	/* input waiting already */
  
! 	if (mousersel)
! 		return 2; /* collision */
! 	mousersel = p;
  	return 0;
  }
  
*** dosfile.c.orig	Mon Sep 13 18:32:46 1993
--- dosfile.c	Mon Sep 13 23:02:06 1993
***************
*** 11,16 ****
--- 11,19 ----
  static long do_dup P_((int,int));
  static void unselectme P_((PROC *));
  
+ /* wait condition for selecting processes which got collisions */
+ short select_coll;
+ 
  /*
   * first, some utility routines
   */
***************
*** 274,279 ****
--- 277,284 ----
  	if (f->dev) {
  		(*f->dev->unselect)(f, (long)p, O_RDONLY);
  		(*f->dev->unselect)(f, (long)p, O_WRONLY);
+ /* give other processes a chance to select on this file */
+ 		wake(SELECT_Q, (long)&select_coll);
  	}
  
  	f->links--;
***************
*** 957,983 ****
  	unsigned timeout;
  	long *rfdp, *wfdp, *xfdp;
  {
! 	long rfd, wfd;
  	long mask, bytes;
  	int i, count;
  	FILEPTR *f;
  	PROC *p;
! 	TIMEOUT *t;
  	short sr;
  
  	if (xfdp)
  		*xfdp = 0;
  
  	if (rfdp) {
! 		rfd = *rfdp; *rfdp = 0;
  	}
  	else
! 		rfd = 0;
  	if (wfdp) {
! 		wfd = *wfdp; *wfdp = 0;
  	}
  	else
! 		wfd = 0;
  
  	TRACE(("Fselect(%u, %lx, %lx)", timeout, rfd, wfd));
  	p = curproc;			/* help the optimizer out */
--- 962,988 ----
  	unsigned timeout;
  	long *rfdp, *wfdp, *xfdp;
  {
! 	long rfd, wfd, orfd, owfd;
  	long mask, bytes;
  	int i, count;
  	FILEPTR *f;
  	PROC *p;
! 	TIMEOUT *t = 0;
  	short sr;
  
  	if (xfdp)
  		*xfdp = 0;
  
  	if (rfdp) {
! 		orfd = rfd = *rfdp; *rfdp = 0;
  	}
  	else
! 		orfd = rfd = 0;
  	if (wfdp) {
! 		owfd = wfd = *wfdp; *wfdp = 0;
  	}
  	else
! 		owfd = wfd = 0;
  
  	TRACE(("Fselect(%u, %lx, %lx)", timeout, rfd, wfd));
  	p = curproc;			/* help the optimizer out */
***************
*** 985,991 ****
  	/* first, validate the masks */
  	mask = 1L;
  	for (i = 0; i < MAX_OPEN; i++) {
! 		if ( ((rfd & mask) || (wfd & mask)) && !(p->handle[i]) ) {
  			DEBUG(("Fselect: invalid handle: %d", i));
  			return EIHNDL;
  		}
--- 990,996 ----
  	/* first, validate the masks */
  	mask = 1L;
  	for (i = 0; i < MAX_OPEN; i++) {
! 		if ( ((orfd & mask) || (owfd & mask)) && !(p->handle[i]) ) {
  			DEBUG(("Fselect: invalid handle: %d", i));
  			return EIHNDL;
  		}
***************
*** 1001,1035 ****
   * closed one of the handles.
   */
  
- 	mask = 1L;
  	count = 0;
  	curproc->wait_cond = (long)wakeselect;		/* flag */
  
  	for (i = 0; i < MAX_OPEN; i++) {
  		if (rfd & mask) {
  			f = p->handle[i];
! 			if ((*f->dev->select)(f, (long)p, O_RDONLY)) {
  				count++;
  				*rfdp |= mask;
  			}
  		}
  		if (wfd & mask) {
  			f = p->handle[i];
! 			if ((*f->dev->select)(f, (long)p, O_WRONLY)) {
  				count++;
  				*wfdp |= mask;
  			}
  		}
  		mask = mask << 1L;
  	}
  
  	if (count == 0) {
! 		/* OK, now let's set a timeout */
  
! 		if (timeout) {
  			t = addtimeout((long)timeout, unselectme);
- 		} else {
- 			t = 0;
  		}
  
  		sr = spl7();
--- 1006,1058 ----
   * closed one of the handles.
   */
  
  	count = 0;
  	curproc->wait_cond = (long)wakeselect;		/* flag */
  
+ 
+ /* NOTE to the above note: since we can get here several times (in
+  * case of collisions) after we've gone to sleep, we probably cannot
+  * assume the filepointers are valid.
+  * I'm not really sure, because `post_sig' sets curproc->wait_cond
+  * to zero and then we cannot get here after we've gone to sleep. 
+  */
+  
+ retry_after_collision:
+ 	mask = 1L;
+ 	
  	for (i = 0; i < MAX_OPEN; i++) {
  		if (rfd & mask) {
  			f = p->handle[i];
! 			if (f) {
! 			    switch ((*f->dev->select)(f, (long)p, O_RDONLY)) {
! 			    case 1:
  				count++;
  				*rfdp |= mask;
+ 			    case 0:
+ 				rfd &= ~mask;
+ 			    }
  			}
  		}
  		if (wfd & mask) {
  			f = p->handle[i];
! 			if (f) {
! 			    switch ((*f->dev->select)(f, (long)p, O_WRONLY)) {
! 			    case 1:
  				count++;
  				*wfdp |= mask;
+ 			    case 0:
+ 				wfd &= ~mask;
+ 			    }
  			}
  		}
  		mask = mask << 1L;
  	}
  
  	if (count == 0) {
! 	/* OK, now let's set a timeout, but only the first time we get here */
  
! 		if (!t && timeout) {
  			t = addtimeout((long)timeout, unselectme);
  		}
  
  		sr = spl7();
***************
*** 1037,1044 ****
  	/* curproc->wait_cond changes when data arrives or the timeout happens */
  		while (curproc->wait_cond == (long)wakeselect) {
  			TRACE(("sleeping in Fselect"));
! 			sleep(SELECT_Q, (long)wakeselect);
  		}
  		spl(sr);
  
  	/* we can cancel the time out now (if it hasn't already happened) */
--- 1060,1076 ----
  	/* curproc->wait_cond changes when data arrives or the timeout happens */
  		while (curproc->wait_cond == (long)wakeselect) {
  			TRACE(("sleeping in Fselect"));
! 			if (rfd || wfd) {
! 				sleep(SELECT_Q, (long)&select_coll);
! 			} else {
! 				sleep(SELECT_Q, (long)wakeselect);
! 			}
  		}
+ 		if (curproc->wait_cond == (long)&select_coll) {
+ 			curproc->wait_cond = (long)wakeselect;
+ 			spl(sr);
+ 			goto retry_after_collision;
+ 		}
  		spl(sr);
  
  	/* we can cancel the time out now (if it hasn't already happened) */
***************
*** 1047,1053 ****
  	/* OK, let's see what data arrived (if any) */
  		mask = 1L;
  		for (i = 0; i < MAX_OPEN; i++) {
! 			if (rfd & mask) {
  				f = p->handle[i];
  				if (f) {
  				    bytes = 1L;
--- 1079,1085 ----
  	/* OK, let's see what data arrived (if any) */
  		mask = 1L;
  		for (i = 0; i < MAX_OPEN; i++) {
! 			if (orfd & mask) {
  				f = p->handle[i];
  				if (f) {
  				    bytes = 1L;
***************
*** 1058,1064 ****
  				    }
  				}
  			}
! 			if (wfd & mask) {
  				f = p->handle[i];
  				if (f) {
  				    bytes = 1L;
--- 1090,1096 ----
  				    }
  				}
  			}
! 			if (owfd & mask) {
  				f = p->handle[i];
  				if (f) {
  				    bytes = 1L;
***************
*** 1071,1089 ****
  			}
  			mask = mask << 1L;
  		}
! 	}
  
  	/* at this point, we either have data or a time out */
  	/* cancel all the selects */
  	mask = 1L;
  
  	for (i = 0; i < MAX_OPEN; i++) {
! 		if (rfd & mask) {
  			f = p->handle[i];
  			if (f)
  				(*f->dev->unselect)(f, (long)p, O_RDONLY);
  		}
! 		if (wfd & mask) {
  			f = p->handle[i];
  			if (f)
  				(*f->dev->unselect)(f, (long)p, O_WRONLY);
--- 1103,1127 ----
  			}
  			mask = mask << 1L;
  		}
! 	} else if (t) {
  
+ 	/* in case data arrived after a collsion, there
+ 	 * could be a timeout pending even if count > 0
+ 	 */
+ 		canceltimeout(t);
+ 	}
+ 	
  	/* at this point, we either have data or a time out */
  	/* cancel all the selects */
  	mask = 1L;
  
  	for (i = 0; i < MAX_OPEN; i++) {
! 		if (orfd & mask) {
  			f = p->handle[i];
  			if (f)
  				(*f->dev->unselect)(f, (long)p, O_RDONLY);
  		}
! 		if (owfd & mask) {
  			f = p->handle[i];
  			if (f)
  				(*f->dev->unselect)(f, (long)p, O_WRONLY);
***************
*** 1091,1096 ****
--- 1129,1138 ----
  		mask = mask << 1L;
  	}
  
+ 	/* wake other processes which got a collision */
+ 	if (orfd || owfd)
+ 		wake(SELECT_Q, (long)&select_coll);
+ 
  	TRACE(("Fselect: returning %d", count));
  	return count;
  }
*** pipefs.c.orig	Mon Sep 13 21:42:50 1993
--- pipefs.c	Mon Sep 13 21:44:34 1993
***************
*** 1010,1018 ****
  			return 1;
  		}
  
! /* BUG: multiple selects fail, only the first one works */
! 		if (!p->rsel)
! 			p->rsel = proc;
  		return 0;
  	} else if (mode == O_WRONLY) {
  		p = (f->flags & O_HEAD) ? this->inp : this->outp;
--- 1010,1018 ----
  			return 1;
  		}
  
! 		if (p->rsel)
! 			return 2; /* collision */
! 		p->rsel = proc;
  		return 0;
  	} else if (mode == O_WRONLY) {
  		p = (f->flags & O_HEAD) ? this->inp : this->outp;
***************
*** 1024,1031 ****
  		if (j >= PIPESIZ) j = 0;
  		if (j != p->head || p->readers <= 0)
  			return 1;	/* data may be written */
! 		if (!p->wsel)
! 			p->wsel = proc;
  		return 0;
  	}
  	return 0;
--- 1024,1032 ----
  		if (j >= PIPESIZ) j = 0;
  		if (j != p->head || p->readers <= 0)
  			return 1;	/* data may be written */
! 		if (p->wsel)
! 			return 2; /* collsion */
! 		p->wsel = proc;
  		return 0;
  	}
  	return 0;
*** proc.c.orig	Mon Sep 13 21:22:00 1993
--- proc.c	Mon Sep 13 21:23:40 1993
***************
*** 608,616 ****
  {
  	PROC *p = (PROC *)param;
  	short s;
! 
  	s = spl7();	/* block interrupts */
! 	if(p->wait_cond == (long)wakeselect) {
  		p->wait_cond = 0;
  	}
  	if (p->wait_q == SELECT_Q) {
--- 608,618 ----
  {
  	PROC *p = (PROC *)param;
  	short s;
! 	extern short select_coll; /* in dosfile.c */
! 	
  	s = spl7();	/* block interrupts */
! 	if(p->wait_cond == (long)wakeselect ||
! 	   p->wait_cond == (long)&select_coll) {
  		p->wait_cond = 0;
  	}
  	if (p->wait_q == SELECT_Q) {
*** signal.c.orig	Fri Sep 17 15:32:58 1993
--- signal.c	Fri Sep 17 15:54:14 1993
***************
*** 56,62 ****
  	int sig;
  {
  	ulong sigm;
! 
  /* if process is ignoring this signal, do nothing
   * also: signal 0 is SIGNULL, and should never be delivered through
   * the normal channels (indeed, it's filtered out in dossig.c,
--- 56,64 ----
  	int sig;
  {
  	ulong sigm;
! 	short sr;
! 	extern short select_coll; /* in dosfile.c */
! 	
  /* if process is ignoring this signal, do nothing
   * also: signal 0 is SIGNULL, and should never be delivered through
   * the normal channels (indeed, it's filtered out in dossig.c,
***************
*** 83,93 ****
  	if ( (p->sigmask & sigm) != 0 )
  		return;
  
  /* otherwise, make sure the process is awake */
  	if (p->wait_q && p->wait_q != READY_Q) {
! 		short sr = spl7();
! 		if (p->wait_q == SELECT_Q)
! 			p->wait_cond = 0;
  		rm_q(p->wait_q, p);
  		add_q(READY_Q, p);
  		spl(sr);
--- 85,104 ----
  	if ( (p->sigmask & sigm) != 0 )
  		return;
  
+ /* Notify p, if p is selecting. After a wake(SELECT_Q, (long)&select_coll)
+  * p might be on READY_Q with p->wait_cond == &select_coll. Posting p a
+  * signal then requires clearing p->wait_cond even if p is not on SELECT_Q.
+  */
+ 	sr = spl7();
+ 	if (p->wait_cond == (long)wakeselect ||
+ 	    p->wait_cond == (long)&select_coll) {
+ 		p->wait_cond = 0;
+ 	}
+ 	spl(sr);
+ 	
  /* otherwise, make sure the process is awake */
  	if (p->wait_q && p->wait_q != READY_Q) {
! 		sr = spl7();
  		rm_q(p->wait_q, p);
  		add_q(READY_Q, p);
  		spl(sr);