Prev: [PATCH 0/2] tty: add EXTPROC support for LINEMODE
Next: [PATCH 0/1] (Was: Fix a race in pid generation that causes pids to be reused immediately.)
From: Howard Chu on 17 Jun 2010 19:50 Howard Chu wrote: > Alan Cox wrote: >>>>> For Alpha this value should match OSF if possible. >>> >>> OSF didn't define this flag, nor did it assign that particular bit to any >>> purpose. Is that good enough? >> >> Fine >> >>>> Are you suggesting that this is completely unfixable/unworkable? Would it be >>>> sufficient to use kernel_termios_to_user_termios() ? >> >> I don't see a way to fix it sanely >> >>>> >>> Actually using kernel_termios_to_user_termios_1(). In all supported >>> architectures this structure is basically aligned with but smaller than the >>> userland struct termios. >> >> The relationship isn't quite so simple and it may change in the future, >> so this seems to be a very bad idea. Besides which syscalls are *cheap* >> so simply notifying someone to reread the terminal data they care about >> should be fine. In that sense it seems SVR4 got it right. > OK. I'm fine with only setting a bit in the packet header, and letting the > application do an ioctl/tcgetattr to discover the actual state. Just deleting that part of the patch was simple enough. The TIOCPKT_IOCTL bit still gets set in the packet header byte; userspace apps will just have to do an ioctl to retrieve the state when the bit is set. I've also added locking to the pty_signal() function. The check for tty->link seems a bit paranoid, but a few other functions do it as well. /* Send a signal to the slave */ static int pty_signal(struct tty_struct *tty, int sig) { unsigned long flags; struct pid *pgrp; if (tty->link) { spin_lock_irqsave(&tty->link->ctrl_lock, flags); pgrp = get_pid(tty->link->pgrp); spin_unlock_irqrestore(&tty->link->ctrl_lock, flags); kill_pgrp(pgrp, sig, 1); put_pid(pgrp); } return 0; } That covers all the feedback so far. I'll be reposting the entire patch again shortly, unless you have any additional thoughts. -- -- Howard Chu CTO, Symas Corp. http://www.symas.com Director, Highland Sun http://highlandsun.com/hyc/ Chief Architect, OpenLDAP http://www.openldap.org/project/ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Howard Chu on 18 Jun 2010 18:30 hyc(a)symas.com wrote: > This patch is against the 2.6.34 source. > @@ -1632,6 +1638,11 @@ static int copy_from_read_buf(struct tty_struct *tty, > spin_lock_irqsave(&tty->read_lock, flags); > tty->read_tail = (tty->read_tail + n)& (N_TTY_BUF_SIZE-1); > tty->read_cnt -= n; > + /* Check if last character is EOF */ > + if (L_EXTPROC(tty)&& tty->icanon) { > + if (!tty->read_cnt&& (*b)[n-1] == EOF_CHAR(tty)) > + n--; > + } > spin_unlock_irqrestore(&tty->read_lock, flags); > *b += n; > *nr -= n; This bit is wrong, only a naked EOF all by itself should be dropped. Should add this to the above: diff --git a/drivers/char/n_tty.c b/drivers/char/n_tty.c index bba123e..428f4fe 100644 --- a/drivers/char/n_tty.c +++ b/drivers/char/n_tty.c @@ -1638,8 +1638,8 @@ static int copy_from_read_buf(struct tty_struct *tty, spin_lock_irqsave(&tty->read_lock, flags); tty->read_tail = (tty->read_tail + n) & (N_TTY_BUF_SIZE-1); tty->read_cnt -= n; - /* Check if last character is EOF */ - if (L_EXTPROC(tty) && tty->icanon) { + /* Turn single EOF into zero-length read */ + if (L_EXTPROC(tty) && tty->icanon && n == 1) { if (!tty->read_cnt && (*b)[n-1] == EOF_CHAR(tty)) n--; } -- -- Howard Chu CTO, Symas Corp. http://www.symas.com Director, Highland Sun http://highlandsun.com/hyc/ Chief Architect, OpenLDAP http://www.openldap.org/project/ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Greg KH on 22 Jun 2010 12:10 On Tue, Jun 22, 2010 at 08:59:38AM -0700, Howard Chu wrote: > Ping... any other issues with this patch? Becides the one that you found yourself? :) Care to resend it with that fixed up? thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Howard Chu on 22 Jun 2010 12:10 Ping... any other issues with this patch? hyc(a)symas.com wrote: > This patch is against the 2.6.34 source. > > Paraphrased from the 1989 BSD patch by David Borman @ cray.com: > > These are the changes needed for the kernel to support > LINEMODE in the server. > > There is a new bit in the termios local flag word, EXTPROC. > When this bit is set, several aspects of the terminal driver > are disabled. Input line editing, character echo, and mapping > of signals are all disabled. This allows the telnetd to turn > off these functions when in linemode, but still keep track of > what state the user wants the terminal to be in. > > New ioctl: > TIOCSIG Generate a signal to processes in the > current process group of the pty. > > There is a new mode for packet driver, the TIOCPKT_IOCTL bit. > When packet mode is turned on in the pty, and the EXTPROC bit > is set, then whenever the state of the pty is changed, the > next read on the master side of the pty will have the TIOCPKT_IOCTL > bit set. This allows the process on the server side of the pty > to know when the state of the terminal has changed; it can then > issue the appropriate ioctl to retrieve the new state. > > Since the original BSD patches accompanied the source code for telnet I've > left that reference here, but obviously the feature is useful for any remote > terminal protocol, including ssh. > > The corresponding feature has existed in the BSD tty driver since 1989. For > historical reference, a good copy of the relevant files can be found here: > > http://anonsvn.mit.edu/viewvc/krb5/trunk/src/appl/telnet/?pathrev=17741 > > Signed-off-by: Howard Chu<hyc(a)symas.com> -- -- Howard Chu CTO, Symas Corp. http://www.symas.com Director, Highland Sun http://highlandsun.com/hyc/ Chief Architect, OpenLDAP http://www.openldap.org/project/ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Alan Cox on 22 Jun 2010 13:30
On Tue, 22 Jun 2010 08:59:38 -0700 Howard Chu <hyc(a)symas.com> wrote: > Ping... any other issues with this patch? Not from me. My only question is 'is it worth doing' but you've apparently answered the question by doing it 8) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |