Re: [PATCH 0/5] SIGWINCH problem with terminal apps still alive

["Adam Tlałka" <atlka@pg.gda.pl>]

[-- Attachment #1: Type: text/plain, Size: 1465 bytes --]

Hello,

actual pty ioctl(,TIOCSWINSZ,) handling is broken IMHO.
I we do that action on normal tty (console for example)
some resize is done or not and resulting size variables are updated
and signal generated. In case of pty ioctl() on slave side it just sets
pty size variables, generates SIGWINCH, but terminal is not changed so
a terminal app will go crazy now. I propose changes which lead to more
consistent handling:

1. set in non pty master/slave case: no changes
2. set in pty master case: we update all sizes and do SIGWINCH on slave
side
3. set in pty slave  case: we update only master variable and do
SIGWINCH on master side
4. read: master reads master variable  while slave reads slave variable

Now if xterm resizes itself then a program on slave gets its signal
but if this program sets terminal sizes by ioctl then only xterm gets
the SIGWINCH signal and could read desired sizes by ioctl and then
resize itself and set valid sizes on slave side by another ioctl() call.
If it not supports this method then there will be no changes on slave
side. I think that it is more proper so on the slave side we will see
always actual values and if terminal resizes we will get SIGWINCH. 

Signed-off-by: Adam Tla/lka <atlka@pg.gda.pl>
 
Regards

-- 
Adam Tlałka       mailto:atlka@pg.gda.pl    ^v^ ^v^ ^v^
System  & Network Administration Group       - - - ~~~~~~
Computer Center, Gdańsk University of Technology, Poland

[-- Attachment #2: 2.6.27_tty_io_5.patch --]
[-- Type: text/x-patch, Size: 3077 bytes --]

--- drivers/char/tty_io_orig.c	2008-10-10 05:37:30.000000000 +0200
+++ drivers/char/tty_io.c	2008-10-16 11:25:53.000000000 +0200
@@ -2490,17 +2490,17 @@ static int tiocsti(struct tty_struct *tt
  *
  *	Copies the kernel idea of the window size into the user buffer.
  *
- *	Locking: tty->termios_mutex is taken to ensure the winsize data
+ *	Locking: real_tty->termios_mutex is taken to ensure the winsize data
  *		is consistent.
  */
 
-static int tiocgwinsz(struct tty_struct *tty, struct winsize __user *arg)
+static int tiocgwinsz(struct tty_struct *tty, struct tty_struct *real_tty, struct winsize __user *arg)
 {
 	int err;
 
-	mutex_lock(&tty->termios_mutex);
+	mutex_lock(&real_tty->termios_mutex);
 	err = copy_to_user(arg, &tty->winsize, sizeof(*arg));
-	mutex_unlock(&tty->termios_mutex);
+	mutex_unlock(&real_tty->termios_mutex);
 
 	return err ? -EFAULT: 0;
 }
@@ -2519,32 +2519,31 @@ static int tiocgwinsz(struct tty_struct 
 int tty_do_resize(struct tty_struct *tty, struct tty_struct *real_tty,
 					struct winsize *ws)
 {
-	struct pid *pgrp, *rpgrp;
+	struct pid *pgrp;
 	unsigned long flags;
 
-	/* For a PTY we need to lock the tty side */
+	/* for a PTY we need to lock the tty side */
 	mutex_lock(&real_tty->termios_mutex);
-	if (!memcmp(ws, &tty->winsize, sizeof(*ws)))
-		goto done;
-	/* Get the PID values and reference them so we can
-	   avoid holding the tty ctrl lock while sending signals */
-	spin_lock_irqsave(&tty->ctrl_lock, flags);
-	pgrp = get_pid(tty->pgrp);
-	rpgrp = get_pid(real_tty->pgrp);
-	spin_unlock_irqrestore(&tty->ctrl_lock, flags);
-
-	if (pgrp)
-		kill_pgrp(pgrp, SIGWINCH, 1);
-	if (rpgrp != pgrp && rpgrp)
-		kill_pgrp(rpgrp, SIGWINCH, 1);
-
-	put_pid(pgrp);
-	put_pid(rpgrp);
-
+	flags = memcmp(ws, &real_tty->winsize, sizeof(*ws));
+	if (tty != real_tty){ /* master side */
+		tty->winsize = *ws;
+		tty = real_tty;
+	} else if (tty->driver->type == TTY_DRIVER_TYPE_PTY){
+		tty = tty->link;
+	}
 	tty->winsize = *ws;
-	real_tty->winsize = *ws;
-done:
 	mutex_unlock(&real_tty->termios_mutex);
+	if (flags){
+		/* Get the PID values and reference them so we can
+		 avoid holding the tty ctrl lock while sending signals */
+		spin_lock_irqsave(&tty->ctrl_lock, flags);
+		pgrp =  get_pid(tty->pgrp);
+		spin_unlock_irqrestore(&tty->ctrl_lock, flags);
+		if (pgrp){
+			kill_pgrp(pgrp, SIGWINCH, 1);
+			put_pid(pgrp);
+		}
+	}
 	return 0;
 }
 
@@ -2570,9 +2569,12 @@ static int tiocswinsz(struct tty_struct 
 	if (copy_from_user(&tmp_ws, arg, sizeof(*arg)))
 		return -EFAULT;
 
-	if (tty->ops->resize)
+	if (tty->ops->resize){
+		if ((tty == real_tty)
+		    && (tty->driver->type == TTY_DRIVER_TYPE_PTY))
+			tty = tty->link;
 		return tty->ops->resize(tty, real_tty, &tmp_ws);
-	else
+	} else
 		return tty_do_resize(tty, real_tty, &tmp_ws);
 }
 
@@ -2996,7 +2998,7 @@ long tty_ioctl(struct file *file, unsign
 	case TIOCSTI:
 		return tiocsti(tty, p);
 	case TIOCGWINSZ:
-		return tiocgwinsz(tty, p);
+		return tiocgwinsz(tty, real_tty, p);
 	case TIOCSWINSZ:
 		return tiocswinsz(tty, real_tty, p);
 	case TIOCCONS: