Re: [PATCH 05/50] tty_ioctl(): start taking TIOC[SG]SERIAL into separate methods

From: Johan Hovold <johan@kernel.org>
To: Al Viro <viro@ZenIV.linux.org.uk>
Cc: Arnd Bergmann <arnd@arndb.de>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 05/50] tty_ioctl(): start taking TIOC[SG]SERIAL into separate methods
Date: Fri, 14 Sep 2018 15:22:16 +0200	[thread overview]
Message-ID: <20180914132216.GC1362@localhost> (raw)
In-Reply-To: <20180913024049.24567-5-viro@ZenIV.linux.org.uk>

On Thu, Sep 13, 2018 at 03:40:04AM +0100, Al Viro wrote:
> From: Al Viro <viro@zeniv.linux.org.uk>
> 
> ->set_serial() and ->get_serial() resp., both taking tty and
> a kernel pointer to serial_struct.
> 
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
>  drivers/tty/tty_io.c       | 36 ++++++++++++++++++++++++++++++------
>  include/linux/tty_driver.h |  3 +++
>  2 files changed, 33 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
> index 2b34ccf269e0..91d73732e11c 100644
> --- a/drivers/tty/tty_io.c
> +++ b/drivers/tty/tty_io.c
> @@ -97,6 +97,7 @@
>  #include <linux/seq_file.h>
>  #include <linux/serial.h>
>  #include <linux/ratelimit.h>
> +#include <linux/compat.h>

Looks like you don't need this (just yet anyway).

>  #include <linux/uaccess.h>
>  
> @@ -2455,22 +2456,40 @@ static int tty_tiocgicount(struct tty_struct *tty, void __user *arg)
>  	return 0;
>  }

[...]

> +static int tty_tiocgserial(struct tty_struct *tty, struct serial_struct __user *ss)
> +{
> +	struct serial_struct v;
> +	int err;

Missing newline.

> +	memset(&v, 0, sizeof(struct serial_struct));
> +
> +	if (!tty->ops->set_serial)

You want get_serial here.

> +		return -ENOIOCTLCMD;
> +	err = tty->ops->get_serial(tty, &v);
> +	if (!err && copy_to_user(ss, &v, sizeof(struct serial_struct)))
> +		err = -EFAULT;
> +	return err;
>  }
>  
>  /*
> @@ -2602,8 +2621,13 @@ long tty_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
>  		}
>  		break;
>  	case TIOCSSERIAL:
> -		tty_warn_deprecated_flags(p);
> -		break;
> +		retval = tty_tiocsserial(tty, p);
> +		if (retval != -ENOIOCTLCMD)
> +			return retval;

Missing break.

> +	case TIOCGSERIAL:
> +		retval = tty_tiocgserial(tty, p);
> +		if (retval != -ENOIOCTLCMD)
> +			return retval;

Missing break.

>  	case TIOCGPTPEER:
>  		/* Special because the struct file is needed */
>  		return ptm_open_peer(file, tty, (int)arg);

Johan