All of lore.kernel.org
 help / color / mirror / Atom feed
From: Howard Chu <hyc@symas.com>
To: Alan Cox <alan@lxorguk.ukuu.org.uk>
Cc: linux-kernel@vger.kernel.org, Greg KH <greg@kroah.com>
Subject: Re: [PATCH] tty: Add EXTPROC support for LINEMODE
Date: Thu, 17 Jun 2010 16:43:01 -0700	[thread overview]
Message-ID: <4C1AB305.5000602@symas.com> (raw)
In-Reply-To: <4C1A923B.4070701@symas.com>

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/

  reply	other threads:[~2010-06-17 23:43 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-06-15 18:56 [PATCH] tty: Add EXTPROC support for LINEMODE hyc
2010-06-15 19:29 ` Alan Cox
2010-06-15 19:54   ` Howard Chu
2010-06-15 20:23     ` Howard Chu
2010-06-16 15:13       ` Derek Fawcus
2010-06-17 20:02     ` Howard Chu
2010-06-17 20:49       ` Alan Cox
2010-06-17 21:23         ` Howard Chu
2010-06-17 23:43           ` Howard Chu [this message]
2010-06-16  1:15   ` Chris Adams
  -- strict thread matches above, loose matches on Subject: below --
2010-06-18 19:45 hyc
2010-06-18 22:20 ` Howard Chu
2010-06-22 15:59 ` Howard Chu
2010-06-22 16:03   ` Greg KH
2010-06-22 17:28   ` Alan Cox

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4C1AB305.5000602@symas.com \
    --to=hyc@symas.com \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=greg@kroah.com \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.