lkml.org 
[lkml]   [2018]   [Sep]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 05/50] tty_ioctl(): start taking TIOC[SG]SERIAL into separate methods
On Fri, Sep 14, 2018 at 03:22:16PM +0200, Johan Hovold wrote:
> 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).

Actually, that should've been added several commits earlier ;-)

> > #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.

Umm... Matter of taste, but since you work in that area more often than
I do... Note, BTW, that tty_tiocgicount() just a bit above that place also
doesn't follow that style. Anyway, I don't care either way - if you want
an empty line in that, no problem.

> > + memset(&v, 0, sizeof(struct serial_struct));
> > +
> > + if (!tty->ops->set_serial)
>
> You want get_serial here.

Nice catch. Fixed.

> > + 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.

*blink*
Right you are... that's a bisect hazard (in the end of the series returns
become unconditional anyway). Fixed.

\
 
 \ /
  Last update: 2018-09-14 17:19    [W:0.829 / U:0.052 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site