From: Arnd Bergmann <arnd@arndb.de>
To: Keith Packard <keithp@keithp.com>, Alan Cox <alan@redhat.com>
Subject: Re: [PATCH] usb/serial: Add compat_ioctl pass-through
Date: Fri, 28 Nov 2008 12:43:02 +0100
User-Agent: KMail/1.9.9
Cc: linux-kernel@vger.kernel.org, Greg Kroah-Hartman <gregkh@suse.de>
References: <1227774576-5503-1-git-send-email-keithp@keithp.com> <200811271531.18498.arnd@arndb.de> <1227810424.4277.55.camel@aiko.keithp.com>
In-Reply-To: <1227810424.4277.55.camel@aiko.keithp.com>
MIME-Version: 1.0
Content-Type: text/plain;
  charset="iso-8859-15"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
Message-Id: <200811281243.02841.arnd@arndb.de>
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2117
Lines: 49

On Thursday 27 November 2008, Keith Packard wrote:
> On Thu, 2008-11-27 at 15:31 +0100, Arnd Bergmann wrote:
>
> > You should probably define compat_ioctl to return an int as well
> > so that it becomes possible to use the same function for both eventually.
> 
> The existing tty layer compat_ioctl is defined to return long; is that
> also wrong?

Yes, this was a slight mistake that got introduced when file_operations->
compat_ioctl first appeared with a long return value. It doesn't really
hurt, but new compat_ioctl functions should just return the 'int' that
gets sent back to user space.

> > Then again, we can also drop the file argument, which is entirely unused
> > in all the usb-serial ioctls.
> 
> I'm not sure it's worth the effort; if some future usb serial ioctl
> needs the argument, we'd end up changing every existing driver back. I
> assume there are some serial devices for which the file is relevant
> during ioctl.

I've just checked all tty device drivers. The only one using the file
pointer in ->ioctl, ->tiocmget and ->tiocmset is serial_core, which
passes it to tty_hung_up_p(). That could easily be changed to check
(tty->flags & TTY_HUPPED) AFAICT.

Maybe Alan can comment on this. I think the code would become cleaner
if we dropped the file argument to these three functions throughout
the tty layer, but it's probably not worth the pain to change themm
call.

> > Maybe the best way for now is to make the new compat_ioctl be
> > int compat_ioctl(struct tty_struct, unsigned int cmd, unsigned long arg);
> > and leave the old ioctl as it is. When we get around to pushing the
> > BKL down into ->ioctl, we can change the prototype at the same time
> > to warn potential out-of-tree drivers.
> 
> Let's figure out what the right compat_ioctl interface is first and
> worry about fixing the existing ioctl interface later.

ok.

	Arnd <><
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/