From: Howard Chu on
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
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
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
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
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/