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 <v...@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 <v...@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.

Reply via email to