[PATCH 00/10] tty: another batch of conversions to guards

[PATCH 00/10] tty: another batch of conversions to guards

[Jiri Slaby (SUSE)]

Hi,

this is also for 6.19 or later.

This series continues to spread guards more both into the tty core code
and drivers.

Jiri Slaby (SUSE) (10):
  tty: pty: use guard()s
  tty: n_tty: use guard()s
  tty: n_hdlc: simplify return from n_hdlc_tty_ioctl()
  tty: n_hdlc: use guard()s
  tty: moxa: use guard()s
  tty: vt/keyboard: use __free()
  tty: vt/keyboard: simplify returns from vt_do_kbkeycode_ioctl()
  tty: vt/keyboard: use guard()s
  serial: serial_core: simplify uart_ioctl() returns
  serial: serial_core: use guard()s

 drivers/tty/moxa.c               | 169 +++++++---------
 drivers/tty/n_hdlc.c             |  79 +++-----
 drivers/tty/n_tty.c              | 109 +++++------
 drivers/tty/pty.c                | 103 +++++-----
 drivers/tty/serial/serial_core.c | 160 ++++++----------
 drivers/tty/vt/keyboard.c        | 318 ++++++++++++-------------------
 6 files changed, 378 insertions(+), 560 deletions(-)

-- 
2.51.1

[PATCH 01/10] tty: pty: use guard()s

[Jiri Slaby (SUSE)]

Use guards in the pty code. This improves readability, makes error
handling easier, and marks locked portions of code explicit. All that
while being sure the lock is unlocked.

pty_set_pktmode() is handled specially -- the conditions are inverted
and return called if conditions unmet. This avoid double nested 'if's.
The variable is renamed to want_pktmode so it is not confused with the
current state of pktmode.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
 drivers/tty/pty.c | 103 ++++++++++++++++++++--------------------------
 1 file changed, 45 insertions(+), 58 deletions(-)

diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c
index 8bb1a01fef2a..76188b8f3ba3 100644
--- a/drivers/tty/pty.c
+++ b/drivers/tty/pty.c
@@ -57,9 +57,8 @@ static void pty_close(struct tty_struct *tty, struct file *filp)
 	set_bit(TTY_IO_ERROR, &tty->flags);
 	wake_up_interruptible(&tty->read_wait);
 	wake_up_interruptible(&tty->write_wait);
-	spin_lock_irq(&tty->ctrl.lock);
-	tty->ctrl.packet = false;
-	spin_unlock_irq(&tty->ctrl.lock);
+	scoped_guard(spinlock_irq, &tty->ctrl.lock)
+		tty->ctrl.packet = false;
 	/* Review - krefs on tty_link ?? */
 	if (!tty->link)
 		return;
@@ -70,10 +69,9 @@ static void pty_close(struct tty_struct *tty, struct file *filp)
 		set_bit(TTY_OTHER_CLOSED, &tty->flags);
 #ifdef CONFIG_UNIX98_PTYS
 		if (tty->driver == ptm_driver) {
-			mutex_lock(&devpts_mutex);
+			guard(mutex)(&devpts_mutex);
 			if (tty->link->driver_data)
 				devpts_pty_kill(tty->link->driver_data);
-			mutex_unlock(&devpts_mutex);
 		}
 #endif
 		tty_vhangup(tty->link);
@@ -157,21 +155,23 @@ static int pty_get_lock(struct tty_struct *tty, int __user *arg)
 /* Set the packet mode on a pty */
 static int pty_set_pktmode(struct tty_struct *tty, int __user *arg)
 {
-	int pktmode;
+	int want_pktmode;
 
-	if (get_user(pktmode, arg))
+	if (get_user(want_pktmode, arg))
 		return -EFAULT;
 
-	spin_lock_irq(&tty->ctrl.lock);
-	if (pktmode) {
-		if (!tty->ctrl.packet) {
-			tty->link->ctrl.pktstatus = 0;
-			smp_mb();
-			tty->ctrl.packet = true;
-		}
-	} else
+	guard(spinlock_irq)(&tty->ctrl.lock);
+	if (!want_pktmode) {
 		tty->ctrl.packet = false;
-	spin_unlock_irq(&tty->ctrl.lock);
+		return 0;
+	}
+
+	if (tty->ctrl.packet)
+		return 0;
+
+	tty->link->ctrl.pktstatus = 0;
+	smp_mb();
+	tty->ctrl.packet = true;
 
 	return 0;
 }
@@ -210,10 +210,9 @@ static void pty_flush_buffer(struct tty_struct *tty)
 
 	tty_buffer_flush(to, NULL);
 	if (to->ctrl.packet) {
-		spin_lock_irq(&tty->ctrl.lock);
+		guard(spinlock_irq)(&tty->ctrl.lock);
 		tty->ctrl.pktstatus |= TIOCPKT_FLUSHWRITE;
 		wake_up_interruptible(&to->read_wait);
-		spin_unlock_irq(&tty->ctrl.lock);
 	}
 }
 
@@ -252,17 +251,17 @@ static void pty_set_termios(struct tty_struct *tty,
 				STOP_CHAR(tty) == '\023' &&
 				START_CHAR(tty) == '\021');
 		if ((old_flow != new_flow) || extproc) {
-			spin_lock_irq(&tty->ctrl.lock);
-			if (old_flow != new_flow) {
-				tty->ctrl.pktstatus &= ~(TIOCPKT_DOSTOP | TIOCPKT_NOSTOP);
-				if (new_flow)
-					tty->ctrl.pktstatus |= TIOCPKT_DOSTOP;
-				else
-					tty->ctrl.pktstatus |= TIOCPKT_NOSTOP;
+			scoped_guard(spinlock_irq, &tty->ctrl.lock) {
+				if (old_flow != new_flow) {
+					tty->ctrl.pktstatus &= ~(TIOCPKT_DOSTOP | TIOCPKT_NOSTOP);
+					if (new_flow)
+						tty->ctrl.pktstatus |= TIOCPKT_DOSTOP;
+					else
+						tty->ctrl.pktstatus |= TIOCPKT_NOSTOP;
+				}
+				if (extproc)
+					tty->ctrl.pktstatus |= TIOCPKT_IOCTL;
 			}
-			if (extproc)
-				tty->ctrl.pktstatus |= TIOCPKT_IOCTL;
-			spin_unlock_irq(&tty->ctrl.lock);
 			wake_up_interruptible(&tty->link->read_wait);
 		}
 	}
@@ -286,9 +285,9 @@ static int pty_resize(struct tty_struct *tty,  struct winsize *ws)
 	struct tty_struct *pty = tty->link;
 
 	/* For a PTY we need to lock the tty side */
-	mutex_lock(&tty->winsize_mutex);
+	guard(mutex)(&tty->winsize_mutex);
 	if (!memcmp(ws, &tty->winsize, sizeof(*ws)))
-		goto done;
+		return 0;
 
 	/* Signal the foreground process group of both ptys */
 	pgrp = tty_get_pgrp(tty);
@@ -304,8 +303,7 @@ static int pty_resize(struct tty_struct *tty,  struct winsize *ws)
 
 	tty->winsize = *ws;
 	pty->winsize = *ws;	/* Never used so will go away soon */
-done:
-	mutex_unlock(&tty->winsize_mutex);
+
 	return 0;
 }
 
@@ -321,28 +319,26 @@ static int pty_resize(struct tty_struct *tty,  struct winsize *ws)
  */
 static void pty_start(struct tty_struct *tty)
 {
-	unsigned long flags;
+	if (!tty->link || !tty->link->ctrl.packet)
+		return;
 
-	if (tty->link && tty->link->ctrl.packet) {
-		spin_lock_irqsave(&tty->ctrl.lock, flags);
+	scoped_guard(spinlock_irqsave, &tty->ctrl.lock) {
 		tty->ctrl.pktstatus &= ~TIOCPKT_STOP;
 		tty->ctrl.pktstatus |= TIOCPKT_START;
-		spin_unlock_irqrestore(&tty->ctrl.lock, flags);
-		wake_up_interruptible_poll(&tty->link->read_wait, EPOLLIN);
 	}
+	wake_up_interruptible_poll(&tty->link->read_wait, EPOLLIN);
 }
 
 static void pty_stop(struct tty_struct *tty)
 {
-	unsigned long flags;
+	if (!tty->link || !tty->link->ctrl.packet)
+		return;
 
-	if (tty->link && tty->link->ctrl.packet) {
-		spin_lock_irqsave(&tty->ctrl.lock, flags);
+	scoped_guard(spinlock_irqsave, &tty->ctrl.lock) {
 		tty->ctrl.pktstatus &= ~TIOCPKT_START;
 		tty->ctrl.pktstatus |= TIOCPKT_STOP;
-		spin_unlock_irqrestore(&tty->ctrl.lock, flags);
-		wake_up_interruptible_poll(&tty->link->read_wait, EPOLLIN);
 	}
+	wake_up_interruptible_poll(&tty->link->read_wait, EPOLLIN);
 }
 
 /**
@@ -705,15 +701,9 @@ static struct tty_struct *ptm_unix98_lookup(struct tty_driver *driver,
 static struct tty_struct *pts_unix98_lookup(struct tty_driver *driver,
 		struct file *file, int idx)
 {
-	struct tty_struct *tty;
-
-	mutex_lock(&devpts_mutex);
-	tty = devpts_get_priv(file->f_path.dentry);
-	mutex_unlock(&devpts_mutex);
+	guard(mutex)(&devpts_mutex);
 	/* Master must be open before slave */
-	if (!tty)
-		return ERR_PTR(-EIO);
-	return tty;
+	return devpts_get_priv(file->f_path.dentry) ? : ERR_PTR(-EIO);
 }
 
 static int pty_unix98_install(struct tty_driver *driver, struct tty_struct *tty)
@@ -811,20 +801,17 @@ static int ptmx_open(struct inode *inode, struct file *filp)
 	}
 
 	/* find a device that is not in use. */
-	mutex_lock(&devpts_mutex);
-	index = devpts_new_index(fsi);
-	mutex_unlock(&devpts_mutex);
+	scoped_guard(mutex, &devpts_mutex)
+		index = devpts_new_index(fsi);
 
 	retval = index;
 	if (index < 0)
 		goto out_put_fsi;
 
 
-	mutex_lock(&tty_mutex);
-	tty = tty_init_dev(ptm_driver, index);
-	/* The tty returned here is locked so we can safely
-	   drop the mutex */
-	mutex_unlock(&tty_mutex);
+	/* The tty returned here is locked so we can safely drop the mutex */
+	scoped_guard(mutex, &tty_mutex)
+		tty = tty_init_dev(ptm_driver, index);
 
 	retval = PTR_ERR(tty);
 	if (IS_ERR(tty))
-- 
2.51.1

[PATCH 02/10] tty: n_tty: use guard()s

[Jiri Slaby (SUSE)]

Use guards in the n_tty code. This improves readability, makes error
handling easier, and marks locked portions of code explicit. All that
while being sure the lock is unlocked.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
 drivers/tty/n_tty.c | 109 +++++++++++++++++++-------------------------
 1 file changed, 48 insertions(+), 61 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 6af3f3a0b531..e6a0f5b40d0a 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -324,12 +324,9 @@ static void reset_buffer_flags(struct n_tty_data *ldata)
 
 static void n_tty_packet_mode_flush(struct tty_struct *tty)
 {
-	unsigned long flags;
-
 	if (tty->link->ctrl.packet) {
-		spin_lock_irqsave(&tty->ctrl.lock, flags);
-		tty->ctrl.pktstatus |= TIOCPKT_FLUSHREAD;
-		spin_unlock_irqrestore(&tty->ctrl.lock, flags);
+		scoped_guard(spinlock_irqsave, &tty->ctrl.lock)
+			tty->ctrl.pktstatus |= TIOCPKT_FLUSHREAD;
 		wake_up_interruptible(&tty->link->read_wait);
 	}
 }
@@ -349,13 +346,12 @@ static void n_tty_packet_mode_flush(struct tty_struct *tty)
  */
 static void n_tty_flush_buffer(struct tty_struct *tty)
 {
-	down_write(&tty->termios_rwsem);
+	guard(rwsem_write)(&tty->termios_rwsem);
 	reset_buffer_flags(tty->disc_data);
 	n_tty_kick_worker(tty);
 
 	if (tty->link)
 		n_tty_packet_mode_flush(tty);
-	up_write(&tty->termios_rwsem);
 }
 
 /**
@@ -737,24 +733,22 @@ static void commit_echoes(struct tty_struct *tty)
 	size_t nr, old, echoed;
 	size_t head;
 
-	mutex_lock(&ldata->output_lock);
-	head = ldata->echo_head;
-	ldata->echo_mark = head;
-	old = ldata->echo_commit - ldata->echo_tail;
-
-	/* Process committed echoes if the accumulated # of bytes
-	 * is over the threshold (and try again each time another
-	 * block is accumulated) */
-	nr = head - ldata->echo_tail;
-	if (nr < ECHO_COMMIT_WATERMARK ||
-	    (nr % ECHO_BLOCK > old % ECHO_BLOCK)) {
-		mutex_unlock(&ldata->output_lock);
-		return;
-	}
+	scoped_guard(mutex, &ldata->output_lock) {
+		head = ldata->echo_head;
+		ldata->echo_mark = head;
+		old = ldata->echo_commit - ldata->echo_tail;
+
+		/*
+		 * Process committed echoes if the accumulated # of bytes is over the threshold
+		 * (and try again each time another block is accumulated)
+		 */
+		nr = head - ldata->echo_tail;
+		if (nr < ECHO_COMMIT_WATERMARK || (nr % ECHO_BLOCK > old % ECHO_BLOCK))
+			return;
 
-	ldata->echo_commit = head;
-	echoed = __process_echoes(tty);
-	mutex_unlock(&ldata->output_lock);
+		ldata->echo_commit = head;
+		echoed = __process_echoes(tty);
+	}
 
 	if (echoed && tty->ops->flush_chars)
 		tty->ops->flush_chars(tty);
@@ -768,10 +762,10 @@ static void process_echoes(struct tty_struct *tty)
 	if (ldata->echo_mark == ldata->echo_tail)
 		return;
 
-	mutex_lock(&ldata->output_lock);
-	ldata->echo_commit = ldata->echo_mark;
-	echoed = __process_echoes(tty);
-	mutex_unlock(&ldata->output_lock);
+	scoped_guard(mutex, &ldata->output_lock) {
+		ldata->echo_commit = ldata->echo_mark;
+		echoed = __process_echoes(tty);
+	}
 
 	if (echoed && tty->ops->flush_chars)
 		tty->ops->flush_chars(tty);
@@ -786,10 +780,9 @@ static void flush_echoes(struct tty_struct *tty)
 	    ldata->echo_commit == ldata->echo_head)
 		return;
 
-	mutex_lock(&ldata->output_lock);
+	guard(mutex)(&ldata->output_lock);
 	ldata->echo_commit = ldata->echo_head;
 	__process_echoes(tty);
-	mutex_unlock(&ldata->output_lock);
 }
 
 /**
@@ -1078,18 +1071,19 @@ static void isig(int sig, struct tty_struct *tty)
 	if (L_NOFLSH(tty)) {
 		/* signal only */
 		__isig(sig, tty);
+		return;
+	}
 
-	} else { /* signal and flush */
-		up_read(&tty->termios_rwsem);
-		down_write(&tty->termios_rwsem);
-
+	/* signal and flush */
+	up_read(&tty->termios_rwsem);
+	scoped_guard(rwsem_write, &tty->termios_rwsem) {
 		__isig(sig, tty);
 
 		/* clear echo buffer */
-		mutex_lock(&ldata->output_lock);
-		ldata->echo_head = ldata->echo_tail = 0;
-		ldata->echo_mark = ldata->echo_commit = 0;
-		mutex_unlock(&ldata->output_lock);
+		scoped_guard(mutex, &ldata->output_lock) {
+			ldata->echo_head = ldata->echo_tail = 0;
+			ldata->echo_mark = ldata->echo_commit = 0;
+		}
 
 		/* clear output buffer */
 		tty_driver_flush_buffer(tty);
@@ -1100,10 +1094,8 @@ static void isig(int sig, struct tty_struct *tty)
 		/* notify pty master of flush */
 		if (tty->link)
 			n_tty_packet_mode_flush(tty);
-
-		up_write(&tty->termios_rwsem);
-		down_read(&tty->termios_rwsem);
 	}
+	down_read(&tty->termios_rwsem);
 }
 
 /**
@@ -1683,7 +1675,7 @@ n_tty_receive_buf_common(struct tty_struct *tty, const u8 *cp, const u8 *fp,
 	size_t n, rcvd = 0;
 	int room, overflow;
 
-	down_read(&tty->termios_rwsem);
+	guard(rwsem_read)(&tty->termios_rwsem);
 
 	do {
 		/*
@@ -1752,8 +1744,6 @@ n_tty_receive_buf_common(struct tty_struct *tty, const u8 *cp, const u8 *fp,
 			n_tty_kick_worker(tty);
 	}
 
-	up_read(&tty->termios_rwsem);
-
 	return rcvd;
 }
 
@@ -1879,10 +1869,9 @@ static void n_tty_close(struct tty_struct *tty)
 	if (tty->link)
 		n_tty_packet_mode_flush(tty);
 
-	down_write(&tty->termios_rwsem);
+	guard(rwsem_write)(&tty->termios_rwsem);
 	vfree(ldata);
 	tty->disc_data = NULL;
-	up_write(&tty->termios_rwsem);
 }
 
 /**
@@ -2247,10 +2236,10 @@ static ssize_t n_tty_read(struct tty_struct *tty, struct file *file, u8 *kbuf,
 			u8 cs;
 			if (kb != kbuf)
 				break;
-			spin_lock_irq(&tty->link->ctrl.lock);
-			cs = tty->link->ctrl.pktstatus;
-			tty->link->ctrl.pktstatus = 0;
-			spin_unlock_irq(&tty->link->ctrl.lock);
+			scoped_guard(spinlock_irq, &tty->link->ctrl.lock) {
+				cs = tty->link->ctrl.pktstatus;
+				tty->link->ctrl.pktstatus = 0;
+			}
 			*kb++ = cs;
 			nr--;
 			break;
@@ -2357,7 +2346,7 @@ static ssize_t n_tty_write(struct tty_struct *tty, struct file *file,
 			return retval;
 	}
 
-	down_read(&tty->termios_rwsem);
+	guard(rwsem_read)(&tty->termios_rwsem);
 
 	/* Write out any echoed characters that are still pending */
 	process_echoes(tty);
@@ -2395,9 +2384,8 @@ static ssize_t n_tty_write(struct tty_struct *tty, struct file *file,
 			struct n_tty_data *ldata = tty->disc_data;
 
 			while (nr > 0) {
-				mutex_lock(&ldata->output_lock);
-				num = tty->ops->write(tty, b, nr);
-				mutex_unlock(&ldata->output_lock);
+				scoped_guard(mutex, &ldata->output_lock)
+					num = tty->ops->write(tty, b, nr);
 				if (num < 0) {
 					retval = num;
 					goto break_out;
@@ -2424,7 +2412,7 @@ static ssize_t n_tty_write(struct tty_struct *tty, struct file *file,
 	remove_wait_queue(&tty->write_wait, &wait);
 	if (nr && tty->fasync)
 		set_bit(TTY_DO_WRITE_WAKEUP, &tty->flags);
-	up_read(&tty->termios_rwsem);
+
 	return (b - buf) ? b - buf : retval;
 }
 
@@ -2498,12 +2486,11 @@ static int n_tty_ioctl(struct tty_struct *tty, unsigned int cmd,
 	case TIOCOUTQ:
 		return put_user(tty_chars_in_buffer(tty), (int __user *) arg);
 	case TIOCINQ:
-		down_write(&tty->termios_rwsem);
-		if (L_ICANON(tty) && !L_EXTPROC(tty))
-			num = inq_canon(ldata);
-		else
-			num = read_cnt(ldata);
-		up_write(&tty->termios_rwsem);
+		scoped_guard(rwsem_write, &tty->termios_rwsem)
+			if (L_ICANON(tty) && !L_EXTPROC(tty))
+				num = inq_canon(ldata);
+			else
+				num = read_cnt(ldata);
 		return put_user(num, (unsigned int __user *) arg);
 	default:
 		return n_tty_ioctl_helper(tty, cmd, arg);
-- 
2.51.1

[PATCH 03/10] tty: n_hdlc: simplify return from n_hdlc_tty_ioctl()

[Jiri Slaby (SUSE)]

The cases in the switch() of n_hdlc_tty_ioctl() can return immediately
-- no need to store into error and return later.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
 drivers/tty/n_hdlc.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/tty/n_hdlc.c b/drivers/tty/n_hdlc.c
index 4a4dc58b866a..66bb8ce6ec40 100644
--- a/drivers/tty/n_hdlc.c
+++ b/drivers/tty/n_hdlc.c
@@ -584,7 +584,6 @@ static int n_hdlc_tty_ioctl(struct tty_struct *tty, unsigned int cmd,
 			    unsigned long arg)
 {
 	struct n_hdlc *n_hdlc = tty->disc_data;
-	int error = 0;
 	int count;
 	unsigned long flags;
 	struct n_hdlc_buf *buf = NULL;
@@ -603,8 +602,7 @@ static int n_hdlc_tty_ioctl(struct tty_struct *tty, unsigned int cmd,
 		else
 			count = 0;
 		spin_unlock_irqrestore(&n_hdlc->rx_buf_list.spinlock, flags);
-		error = put_user(count, (int __user *)arg);
-		break;
+		return put_user(count, (int __user *)arg);
 
 	case TIOCOUTQ:
 		/* get the pending tx byte count in the driver */
@@ -616,8 +614,7 @@ static int n_hdlc_tty_ioctl(struct tty_struct *tty, unsigned int cmd,
 		if (buf)
 			count += buf->count;
 		spin_unlock_irqrestore(&n_hdlc->tx_buf_list.spinlock, flags);
-		error = put_user(count, (int __user *)arg);
-		break;
+		return put_user(count, (int __user *)arg);
 
 	case TCFLSH:
 		switch (arg) {
@@ -628,11 +625,8 @@ static int n_hdlc_tty_ioctl(struct tty_struct *tty, unsigned int cmd,
 		fallthrough;	/* to default */
 
 	default:
-		error = n_tty_ioctl_helper(tty, cmd, arg);
-		break;
+		return n_tty_ioctl_helper(tty, cmd, arg);
 	}
-	return error;
-
 }	/* end of n_hdlc_tty_ioctl() */
 
 /**
-- 
2.51.1

[PATCH 04/10] tty: n_hdlc: use guard()s

[Jiri Slaby (SUSE)]

Use guards in the n_hdlc code. This improves readability, makes error
handling easier, and marks locked portions of code explicit. All that
while being sure the lock is unlocked.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
 drivers/tty/n_hdlc.c | 67 +++++++++++++++++---------------------------
 1 file changed, 26 insertions(+), 41 deletions(-)

diff --git a/drivers/tty/n_hdlc.c b/drivers/tty/n_hdlc.c
index 66bb8ce6ec40..3c9dcb0928c6 100644
--- a/drivers/tty/n_hdlc.c
+++ b/drivers/tty/n_hdlc.c
@@ -263,21 +263,18 @@ static int n_hdlc_tty_open(struct tty_struct *tty)
  */
 static void n_hdlc_send_frames(struct n_hdlc *n_hdlc, struct tty_struct *tty)
 {
-	unsigned long flags;
 	struct n_hdlc_buf *tbuf;
 	ssize_t actual;
 
 check_again:
-
-	spin_lock_irqsave(&n_hdlc->tx_buf_list.spinlock, flags);
-	if (n_hdlc->tbusy) {
-		n_hdlc->woke_up = true;
-		spin_unlock_irqrestore(&n_hdlc->tx_buf_list.spinlock, flags);
-		return;
+	scoped_guard(spinlock_irqsave, &n_hdlc->tx_buf_list.spinlock) {
+		if (n_hdlc->tbusy) {
+			n_hdlc->woke_up = true;
+			return;
+		}
+		n_hdlc->tbusy = true;
+		n_hdlc->woke_up = false;
 	}
-	n_hdlc->tbusy = true;
-	n_hdlc->woke_up = false;
-	spin_unlock_irqrestore(&n_hdlc->tx_buf_list.spinlock, flags);
 
 	tbuf = n_hdlc_buf_get(&n_hdlc->tx_buf_list);
 	while (tbuf) {
@@ -324,9 +321,8 @@ static void n_hdlc_send_frames(struct n_hdlc *n_hdlc, struct tty_struct *tty)
 		clear_bit(TTY_DO_WRITE_WAKEUP, &tty->flags);
 
 	/* Clear the re-entry flag */
-	spin_lock_irqsave(&n_hdlc->tx_buf_list.spinlock, flags);
-	n_hdlc->tbusy = false;
-	spin_unlock_irqrestore(&n_hdlc->tx_buf_list.spinlock, flags);
+	scoped_guard(spinlock_irqsave, &n_hdlc->tx_buf_list.spinlock)
+		n_hdlc->tbusy = false;
 
 	if (n_hdlc->woke_up)
 		goto check_again;
@@ -585,7 +581,6 @@ static int n_hdlc_tty_ioctl(struct tty_struct *tty, unsigned int cmd,
 {
 	struct n_hdlc *n_hdlc = tty->disc_data;
 	int count;
-	unsigned long flags;
 	struct n_hdlc_buf *buf = NULL;
 
 	pr_debug("%s() called %d\n", __func__, cmd);
@@ -594,26 +589,26 @@ static int n_hdlc_tty_ioctl(struct tty_struct *tty, unsigned int cmd,
 	case FIONREAD:
 		/* report count of read data available */
 		/* in next available frame (if any) */
-		spin_lock_irqsave(&n_hdlc->rx_buf_list.spinlock, flags);
-		buf = list_first_entry_or_null(&n_hdlc->rx_buf_list.list,
-						struct n_hdlc_buf, list_item);
-		if (buf)
-			count = buf->count;
-		else
-			count = 0;
-		spin_unlock_irqrestore(&n_hdlc->rx_buf_list.spinlock, flags);
+		scoped_guard(spinlock_irqsave, &n_hdlc->rx_buf_list.spinlock) {
+			buf = list_first_entry_or_null(&n_hdlc->rx_buf_list.list,
+						       struct n_hdlc_buf, list_item);
+			if (buf)
+				count = buf->count;
+			else
+				count = 0;
+		}
 		return put_user(count, (int __user *)arg);
 
 	case TIOCOUTQ:
 		/* get the pending tx byte count in the driver */
 		count = tty_chars_in_buffer(tty);
 		/* add size of next output frame in queue */
-		spin_lock_irqsave(&n_hdlc->tx_buf_list.spinlock, flags);
-		buf = list_first_entry_or_null(&n_hdlc->tx_buf_list.list,
-						struct n_hdlc_buf, list_item);
-		if (buf)
-			count += buf->count;
-		spin_unlock_irqrestore(&n_hdlc->tx_buf_list.spinlock, flags);
+		scoped_guard(spinlock_irqsave, &n_hdlc->tx_buf_list.spinlock) {
+			buf = list_first_entry_or_null(&n_hdlc->tx_buf_list.list,
+						       struct n_hdlc_buf, list_item);
+			if (buf)
+				count += buf->count;
+		}
 		return put_user(count, (int __user *)arg);
 
 	case TCFLSH:
@@ -720,14 +715,10 @@ static struct n_hdlc *n_hdlc_alloc(void)
 static void n_hdlc_buf_return(struct n_hdlc_buf_list *buf_list,
 						struct n_hdlc_buf *buf)
 {
-	unsigned long flags;
-
-	spin_lock_irqsave(&buf_list->spinlock, flags);
+	guard(spinlock_irqsave)(&buf_list->spinlock);
 
 	list_add(&buf->list_item, &buf_list->list);
 	buf_list->count++;
-
-	spin_unlock_irqrestore(&buf_list->spinlock, flags);
 }
 
 /**
@@ -738,14 +729,10 @@ static void n_hdlc_buf_return(struct n_hdlc_buf_list *buf_list,
 static void n_hdlc_buf_put(struct n_hdlc_buf_list *buf_list,
 			   struct n_hdlc_buf *buf)
 {
-	unsigned long flags;
-
-	spin_lock_irqsave(&buf_list->spinlock, flags);
+	guard(spinlock_irqsave)(&buf_list->spinlock);
 
 	list_add_tail(&buf->list_item, &buf_list->list);
 	buf_list->count++;
-
-	spin_unlock_irqrestore(&buf_list->spinlock, flags);
 }	/* end of n_hdlc_buf_put() */
 
 /**
@@ -758,10 +745,9 @@ static void n_hdlc_buf_put(struct n_hdlc_buf_list *buf_list,
  */
 static struct n_hdlc_buf *n_hdlc_buf_get(struct n_hdlc_buf_list *buf_list)
 {
-	unsigned long flags;
 	struct n_hdlc_buf *buf;
 
-	spin_lock_irqsave(&buf_list->spinlock, flags);
+	guard(spinlock_irqsave)(&buf_list->spinlock);
 
 	buf = list_first_entry_or_null(&buf_list->list,
 						struct n_hdlc_buf, list_item);
@@ -770,7 +756,6 @@ static struct n_hdlc_buf *n_hdlc_buf_get(struct n_hdlc_buf_list *buf_list)
 		buf_list->count--;
 	}
 
-	spin_unlock_irqrestore(&buf_list->spinlock, flags);
 	return buf;
 }	/* end of n_hdlc_buf_get() */
 
-- 
2.51.1

[PATCH 05/10] tty: moxa: use guard()s

[Jiri Slaby (SUSE)]

Use guards in the moxa code. This improves readability, makes error
handling easier, and marks locked portions of code explicit. All that
while being sure the lock is unlocked.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
 drivers/tty/moxa.c | 169 ++++++++++++++++++++-------------------------
 1 file changed, 73 insertions(+), 96 deletions(-)

diff --git a/drivers/tty/moxa.c b/drivers/tty/moxa.c
index 329b30fac8fc..1bb2376af85c 100644
--- a/drivers/tty/moxa.c
+++ b/drivers/tty/moxa.c
@@ -487,25 +487,20 @@ static void moxa_wait_finish(void __iomem *ofsAddr)
 
 static void moxafunc(void __iomem *ofsAddr, u16 cmd, u16 arg)
 {
-        unsigned long flags;
-        spin_lock_irqsave(&moxafunc_lock, flags);
+	guard(spinlock_irqsave)(&moxafunc_lock);
 	writew(arg, ofsAddr + FuncArg);
 	writew(cmd, ofsAddr + FuncCode);
 	moxa_wait_finish(ofsAddr);
-	spin_unlock_irqrestore(&moxafunc_lock, flags);
 }
 
 static int moxafuncret(void __iomem *ofsAddr, u16 cmd, u16 arg)
 {
-        unsigned long flags;
-        u16 ret;
-        spin_lock_irqsave(&moxafunc_lock, flags);
+	guard(spinlock_irqsave)(&moxafunc_lock);
 	writew(arg, ofsAddr + FuncArg);
 	writew(cmd, ofsAddr + FuncCode);
 	moxa_wait_finish(ofsAddr);
-	ret = readw(ofsAddr + FuncArg);
-	spin_unlock_irqrestore(&moxafunc_lock, flags);
-	return ret;
+
+	return readw(ofsAddr + FuncArg);
 }
 
 static void moxa_low_water_check(void __iomem *ofsAddr)
@@ -1002,11 +997,11 @@ static int moxa_init_board(struct moxa_board_conf *brd, struct device *dev)
 	if (ret)
 		goto err_free;
 
-	spin_lock_bh(&moxa_lock);
-	brd->ready = 1;
-	if (!timer_pending(&moxaTimer))
-		mod_timer(&moxaTimer, jiffies + HZ / 50);
-	spin_unlock_bh(&moxa_lock);
+	scoped_guard(spinlock_bh, &moxa_lock) {
+		brd->ready = 1;
+		if (!timer_pending(&moxaTimer))
+			mod_timer(&moxaTimer, jiffies + HZ / 50);
+	}
 
 	first_idx = (brd - moxa_boards) * MAX_PORTS_PER_BOARD;
 	for (i = 0; i < brd->numPorts; i++)
@@ -1026,29 +1021,29 @@ static void moxa_board_deinit(struct moxa_board_conf *brd)
 {
 	unsigned int a, opened, first_idx;
 
-	mutex_lock(&moxa_openlock);
-	spin_lock_bh(&moxa_lock);
-	brd->ready = 0;
-	spin_unlock_bh(&moxa_lock);
-
-	/* pci hot-un-plug support */
-	for (a = 0; a < brd->numPorts; a++)
-		if (tty_port_initialized(&brd->ports[a].port))
-			tty_port_tty_hangup(&brd->ports[a].port, false);
-
-	for (a = 0; a < MAX_PORTS_PER_BOARD; a++)
-		tty_port_destroy(&brd->ports[a].port);
+	scoped_guard(mutex, &moxa_openlock) {
+		scoped_guard(spinlock_bh, &moxa_lock)
+			brd->ready = 0;
 
-	while (1) {
-		opened = 0;
+		/* pci hot-un-plug support */
 		for (a = 0; a < brd->numPorts; a++)
 			if (tty_port_initialized(&brd->ports[a].port))
-				opened++;
-		mutex_unlock(&moxa_openlock);
-		if (!opened)
-			break;
-		msleep(50);
-		mutex_lock(&moxa_openlock);
+				tty_port_tty_hangup(&brd->ports[a].port, false);
+
+		for (a = 0; a < MAX_PORTS_PER_BOARD; a++)
+			tty_port_destroy(&brd->ports[a].port);
+
+		while (1) {
+			opened = 0;
+			for (a = 0; a < brd->numPorts; a++)
+				if (tty_port_initialized(&brd->ports[a].port))
+					opened++;
+			if (!opened)
+				break;
+			mutex_unlock(&moxa_openlock);
+			msleep(50);
+			mutex_lock(&moxa_openlock);
+		}
 	}
 
 	first_idx = (brd - moxa_boards) * MAX_PORTS_PER_BOARD;
@@ -1206,12 +1201,9 @@ static void moxa_shutdown(struct tty_port *port)
 static bool moxa_carrier_raised(struct tty_port *port)
 {
 	struct moxa_port *ch = container_of(port, struct moxa_port, port);
-	int dcd;
 
-	spin_lock_irq(&port->lock);
-	dcd = ch->DCDState;
-	spin_unlock_irq(&port->lock);
-	return dcd;
+	guard(spinlock_irq)(&port->lock);
+	return ch->DCDState;
 }
 
 static void moxa_dtr_rts(struct tty_port *port, bool active)
@@ -1225,37 +1217,31 @@ static int moxa_open(struct tty_struct *tty, struct file *filp)
 {
 	struct moxa_board_conf *brd;
 	struct moxa_port *ch;
-	int port;
-
-	port = tty->index;
-	if (mutex_lock_interruptible(&moxa_openlock))
-		return -ERESTARTSYS;
-	brd = &moxa_boards[port / MAX_PORTS_PER_BOARD];
-	if (!brd->ready) {
-		mutex_unlock(&moxa_openlock);
-		return -ENODEV;
-	}
+	int port = tty->index;
 
-	if (port % MAX_PORTS_PER_BOARD >= brd->numPorts) {
-		mutex_unlock(&moxa_openlock);
-		return -ENODEV;
-	}
-
-	ch = &brd->ports[port % MAX_PORTS_PER_BOARD];
-	ch->port.count++;
-	tty->driver_data = ch;
-	tty_port_tty_set(&ch->port, tty);
-	mutex_lock(&ch->port.mutex);
-	if (!tty_port_initialized(&ch->port)) {
-		ch->statusflags = 0;
-		moxa_set_tty_param(tty, &tty->termios);
-		MoxaPortLineCtrl(ch, true, true);
-		MoxaPortEnable(ch);
-		MoxaSetFifo(ch, ch->type == PORT_16550A);
-		tty_port_set_initialized(&ch->port, true);
+	scoped_cond_guard(mutex_intr, return -ERESTARTSYS, &moxa_openlock) {
+		brd = &moxa_boards[port / MAX_PORTS_PER_BOARD];
+		if (!brd->ready)
+			return -ENODEV;
+
+		if (port % MAX_PORTS_PER_BOARD >= brd->numPorts)
+			return -ENODEV;
+
+		ch = &brd->ports[port % MAX_PORTS_PER_BOARD];
+		ch->port.count++;
+		tty->driver_data = ch;
+		tty_port_tty_set(&ch->port, tty);
+
+		guard(mutex)(&ch->port.mutex);
+		if (!tty_port_initialized(&ch->port)) {
+			ch->statusflags = 0;
+			moxa_set_tty_param(tty, &tty->termios);
+			MoxaPortLineCtrl(ch, true, true);
+			MoxaPortEnable(ch);
+			MoxaSetFifo(ch, ch->type == PORT_16550A);
+			tty_port_set_initialized(&ch->port, true);
+		}
 	}
-	mutex_unlock(&ch->port.mutex);
-	mutex_unlock(&moxa_openlock);
 
 	return tty_port_block_til_ready(&ch->port, tty, filp);
 }
@@ -1270,15 +1256,13 @@ static void moxa_close(struct tty_struct *tty, struct file *filp)
 static ssize_t moxa_write(struct tty_struct *tty, const u8 *buf, size_t count)
 {
 	struct moxa_port *ch = tty->driver_data;
-	unsigned long flags;
 	int len;
 
 	if (ch == NULL)
 		return 0;
 
-	spin_lock_irqsave(&moxa_lock, flags);
-	len = MoxaPortWriteData(tty, buf, count);
-	spin_unlock_irqrestore(&moxa_lock, flags);
+	scoped_guard(spinlock_irqsave, &moxa_lock)
+		len = MoxaPortWriteData(tty, buf, count);
 
 	set_bit(LOWWAIT, &ch->statusflags);
 	return len;
@@ -1349,12 +1333,10 @@ static int moxa_tiocmset(struct tty_struct *tty,
 	bool dtr_active, rts_active;
 	struct moxa_port *ch;
 
-	mutex_lock(&moxa_openlock);
+	guard(mutex)(&moxa_openlock);
 	ch = tty->driver_data;
-	if (!ch) {
-		mutex_unlock(&moxa_openlock);
+	if (!ch)
 		return -EINVAL;
-	}
 
 	MoxaPortGetLineOut(ch, &dtr_active, &rts_active);
 	if (set & TIOCM_RTS)
@@ -1366,7 +1348,7 @@ static int moxa_tiocmset(struct tty_struct *tty,
 	if (clear & TIOCM_DTR)
 		dtr_active = false;
 	MoxaPortLineCtrl(ch, dtr_active, rts_active);
-	mutex_unlock(&moxa_openlock);
+
 	return 0;
 }
 
@@ -1415,18 +1397,17 @@ static void moxa_hangup(struct tty_struct *tty)
 
 static void moxa_new_dcdstate(struct moxa_port *p, u8 dcd)
 {
-	unsigned long flags;
 	dcd = !!dcd;
 
-	spin_lock_irqsave(&p->port.lock, flags);
-	if (dcd != p->DCDState) {
-        	p->DCDState = dcd;
-        	spin_unlock_irqrestore(&p->port.lock, flags);
-		if (!dcd)
-			tty_port_tty_hangup(&p->port, true);
+	scoped_guard(spinlock_irqsave, &p->port.lock) {
+		if (dcd == p->DCDState)
+			return;
+
+		p->DCDState = dcd;
 	}
-	else
-		spin_unlock_irqrestore(&p->port.lock, flags);
+
+	if (!dcd)
+		tty_port_tty_hangup(&p->port, true);
 }
 
 static int moxa_poll_port(struct moxa_port *p, unsigned int handle,
@@ -1494,7 +1475,7 @@ static void moxa_poll(struct timer_list *unused)
 	u16 __iomem *ip;
 	unsigned int card, port, served = 0;
 
-	spin_lock(&moxa_lock);
+	guard(spinlock)(&moxa_lock);
 	for (card = 0; card < MAX_BOARDS; card++) {
 		brd = &moxa_boards[card];
 		if (!brd->ready)
@@ -1525,7 +1506,6 @@ static void moxa_poll(struct timer_list *unused)
 
 	if (served)
 		mod_timer(&moxaTimer, jiffies + HZ / 50);
-	spin_unlock(&moxa_lock);
 }
 
 /******************************************************************************/
@@ -1861,13 +1841,11 @@ static int MoxaPortSetTermio(struct moxa_port *port, struct ktermios *termio,
 	baud = MoxaPortSetBaud(port, baud);
 
 	if (termio->c_iflag & (IXON | IXOFF | IXANY)) {
-	        spin_lock_irq(&moxafunc_lock);
+		guard(spinlock_irq)(&moxafunc_lock);
 		writeb(termio->c_cc[VSTART], ofsAddr + FuncArg);
 		writeb(termio->c_cc[VSTOP], ofsAddr + FuncArg1);
 		writeb(FC_SetXonXoff, ofsAddr + FuncCode);
 		moxa_wait_finish(ofsAddr);
-		spin_unlock_irq(&moxafunc_lock);
-
 	}
 	return baud;
 }
@@ -2098,13 +2076,13 @@ static int moxa_get_serial_info(struct tty_struct *tty,
 
 	if (!info)
 		return -ENODEV;
-	mutex_lock(&info->port.mutex);
+	guard(mutex)(&info->port.mutex);
 	ss->type = info->type;
 	ss->line = info->port.tty->index;
 	ss->flags = info->port.flags;
 	ss->baud_base = 921600;
 	ss->close_delay = jiffies_to_msecs(info->port.close_delay) / 10;
-	mutex_unlock(&info->port.mutex);
+
 	return 0;
 }
 
@@ -2120,13 +2098,12 @@ static int moxa_set_serial_info(struct tty_struct *tty,
 
 	close_delay = msecs_to_jiffies(ss->close_delay * 10);
 
-	mutex_lock(&info->port.mutex);
+	guard(mutex)(&info->port.mutex);
 	if (!capable(CAP_SYS_ADMIN)) {
 		if (close_delay != info->port.close_delay ||
 		    ss->type != info->type ||
 		    ((ss->flags & ~ASYNC_USR_MASK) !=
 		     (info->port.flags & ~ASYNC_USR_MASK))) {
-			mutex_unlock(&info->port.mutex);
 			return -EPERM;
 		}
 	} else {
@@ -2136,7 +2113,7 @@ static int moxa_set_serial_info(struct tty_struct *tty,
 
 		info->type = ss->type;
 	}
-	mutex_unlock(&info->port.mutex);
+
 	return 0;
 }
 
-- 
2.51.1

[PATCH 06/10] tty: vt/keyboard: use __free()

[Jiri Slaby (SUSE)]

The vt/keyboard code can use __free to ensure the temporary buffers are
freed. Perform the switch.

And even one non-temporary in kbd_connect(). There are fail paths, so
ensure the buffer is freed in them and not when returning 0 -- by
retain_and_null_ptr().

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
 drivers/tty/vt/keyboard.c | 90 ++++++++++++++++-----------------------
 1 file changed, 37 insertions(+), 53 deletions(-)

diff --git a/drivers/tty/vt/keyboard.c b/drivers/tty/vt/keyboard.c
index ee1d9c448c7e..65913a137862 100644
--- a/drivers/tty/vt/keyboard.c
+++ b/drivers/tty/vt/keyboard.c
@@ -1566,10 +1566,9 @@ static bool kbd_match(struct input_handler *handler, struct input_dev *dev)
 static int kbd_connect(struct input_handler *handler, struct input_dev *dev,
 			const struct input_device_id *id)
 {
-	struct input_handle *handle;
 	int error;
 
-	handle = kzalloc(sizeof(struct input_handle), GFP_KERNEL);
+	struct input_handle __free(kfree) *handle = kzalloc(sizeof(*handle), GFP_KERNEL);
 	if (!handle)
 		return -ENOMEM;
 
@@ -1579,18 +1578,18 @@ static int kbd_connect(struct input_handler *handler, struct input_dev *dev,
 
 	error = input_register_handle(handle);
 	if (error)
-		goto err_free_handle;
+		return error;
 
 	error = input_open_device(handle);
 	if (error)
 		goto err_unregister_handle;
 
+	retain_and_null_ptr(handle);
+
 	return 0;
 
  err_unregister_handle:
 	input_unregister_handle(handle);
- err_free_handle:
-	kfree(handle);
 	return error;
 }
 
@@ -1683,17 +1682,15 @@ int vt_do_diacrit(unsigned int cmd, void __user *udp, int perm)
 {
 	unsigned long flags;
 	int asize;
-	int ret = 0;
 
 	switch (cmd) {
 	case KDGKBDIACR:
 	{
 		struct kbdiacrs __user *a = udp;
-		struct kbdiacr *dia;
 		int i;
 
-		dia = kmalloc_array(MAX_DIACR, sizeof(struct kbdiacr),
-								GFP_KERNEL);
+		struct kbdiacr __free(kfree) *dia = kmalloc_array(MAX_DIACR, sizeof(struct kbdiacr),
+								  GFP_KERNEL);
 		if (!dia)
 			return -ENOMEM;
 
@@ -1713,20 +1710,17 @@ int vt_do_diacrit(unsigned int cmd, void __user *udp, int perm)
 		spin_unlock_irqrestore(&kbd_event_lock, flags);
 
 		if (put_user(asize, &a->kb_cnt))
-			ret = -EFAULT;
-		else  if (copy_to_user(a->kbdiacr, dia,
-				asize * sizeof(struct kbdiacr)))
-			ret = -EFAULT;
-		kfree(dia);
-		return ret;
+			return -EFAULT;
+		if (copy_to_user(a->kbdiacr, dia, asize * sizeof(struct kbdiacr)))
+			return -EFAULT;
+		return 0;
 	}
 	case KDGKBDIACRUC:
 	{
 		struct kbdiacrsuc __user *a = udp;
-		void *buf;
 
-		buf = kmalloc_array(MAX_DIACR, sizeof(struct kbdiacruc),
-								GFP_KERNEL);
+		void __free(kfree) *buf = kmalloc_array(MAX_DIACR, sizeof(struct kbdiacruc),
+							GFP_KERNEL);
 		if (buf == NULL)
 			return -ENOMEM;
 
@@ -1740,18 +1734,17 @@ int vt_do_diacrit(unsigned int cmd, void __user *udp, int perm)
 		spin_unlock_irqrestore(&kbd_event_lock, flags);
 
 		if (put_user(asize, &a->kb_cnt))
-			ret = -EFAULT;
-		else if (copy_to_user(a->kbdiacruc, buf,
-				asize*sizeof(struct kbdiacruc)))
-			ret = -EFAULT;
-		kfree(buf);
-		return ret;
+			return -EFAULT;
+		if (copy_to_user(a->kbdiacruc, buf, asize * sizeof(struct kbdiacruc)))
+			return -EFAULT;
+
+		return 0;
 	}
 
 	case KDSKBDIACR:
 	{
 		struct kbdiacrs __user *a = udp;
-		struct kbdiacr *dia = NULL;
+		struct kbdiacr __free(kfree) *dia = NULL;
 		unsigned int ct;
 		int i;
 
@@ -1780,7 +1773,7 @@ int vt_do_diacrit(unsigned int cmd, void __user *udp, int perm)
 					conv_8bit_to_uni(dia[i].result);
 		}
 		spin_unlock_irqrestore(&kbd_event_lock, flags);
-		kfree(dia);
+
 		return 0;
 	}
 
@@ -1788,7 +1781,7 @@ int vt_do_diacrit(unsigned int cmd, void __user *udp, int perm)
 	{
 		struct kbdiacrsuc __user *a = udp;
 		unsigned int ct;
-		void *buf = NULL;
+		void __free(kfree) *buf = NULL;
 
 		if (!perm)
 			return -EPERM;
@@ -1811,11 +1804,10 @@ int vt_do_diacrit(unsigned int cmd, void __user *udp, int perm)
 					ct * sizeof(struct kbdiacruc));
 		accent_table_size = ct;
 		spin_unlock_irqrestore(&kbd_event_lock, flags);
-		kfree(buf);
 		return 0;
 	}
 	}
-	return ret;
+	return 0;
 }
 
 /**
@@ -1934,7 +1926,7 @@ static int vt_kdskbent(unsigned char kbdmode, unsigned char idx,
 		unsigned char map, unsigned short val)
 {
 	unsigned long flags;
-	unsigned short *key_map, *new_map, oldval;
+	unsigned short *key_map, oldval;
 
 	if (!idx && val == K_NOSUCHMAP) {
 		spin_lock_irqsave(&kbd_event_lock, flags);
@@ -1965,7 +1957,7 @@ static int vt_kdskbent(unsigned char kbdmode, unsigned char idx,
 		return 0;
 #endif
 
-	new_map = kmalloc(sizeof(plain_map), GFP_KERNEL);
+	unsigned short __free(kfree) *new_map = kmalloc(sizeof(plain_map), GFP_KERNEL);
 	if (!new_map)
 		return -ENOMEM;
 
@@ -1977,17 +1969,14 @@ static int vt_kdskbent(unsigned char kbdmode, unsigned char idx,
 		if (keymap_count >= MAX_NR_OF_USER_KEYMAPS &&
 		    !capable(CAP_SYS_RESOURCE)) {
 			spin_unlock_irqrestore(&kbd_event_lock, flags);
-			kfree(new_map);
 			return -EPERM;
 		}
-		key_maps[map] = new_map;
-		key_map = new_map;
+		key_map = key_maps[map] = no_free_ptr(new_map);
 		key_map[0] = U(K_ALLOCATED);
 		for (j = 1; j < NR_KEYS; j++)
 			key_map[j] = U(K_HOLE);
 		keymap_count++;
-	} else
-		kfree(new_map);
+	}
 
 	oldval = U(key_map[idx]);
 	if (val == oldval)
@@ -2050,8 +2039,6 @@ int vt_do_kdgkb_ioctl(int cmd, struct kbsentry __user *user_kdgkb, int perm)
 {
 	unsigned char kb_func;
 	unsigned long flags;
-	char *kbs;
-	int ret;
 
 	if (get_user(kb_func, &user_kdgkb->kb_func))
 		return -EFAULT;
@@ -2063,7 +2050,7 @@ int vt_do_kdgkb_ioctl(int cmd, struct kbsentry __user *user_kdgkb, int perm)
 		/* size should have been a struct member */
 		ssize_t len = sizeof(user_kdgkb->kb_string);
 
-		kbs = kmalloc(len, GFP_KERNEL);
+		char __free(kfree) *kbs = kmalloc(len, GFP_KERNEL);
 		if (!kbs)
 			return -ENOMEM;
 
@@ -2071,20 +2058,20 @@ int vt_do_kdgkb_ioctl(int cmd, struct kbsentry __user *user_kdgkb, int perm)
 		len = strscpy(kbs, func_table[kb_func] ? : "", len);
 		spin_unlock_irqrestore(&func_buf_lock, flags);
 
-		if (len < 0) {
-			ret = -ENOSPC;
-			break;
-		}
-		ret = copy_to_user(user_kdgkb->kb_string, kbs, len + 1) ?
-			-EFAULT : 0;
-		break;
+		if (len < 0)
+			return -ENOSPC;
+
+		if (copy_to_user(user_kdgkb->kb_string, kbs, len + 1))
+			return -EFAULT;
+
+		return 0;
 	}
 	case KDSKBSENT:
 		if (!perm || !capable(CAP_SYS_TTY_CONFIG))
 			return -EPERM;
 
-		kbs = strndup_user(user_kdgkb->kb_string,
-				sizeof(user_kdgkb->kb_string));
+		char __free(kfree) *kbs = strndup_user(user_kdgkb->kb_string,
+						       sizeof(user_kdgkb->kb_string));
 		if (IS_ERR(kbs))
 			return PTR_ERR(kbs);
 
@@ -2092,13 +2079,10 @@ int vt_do_kdgkb_ioctl(int cmd, struct kbsentry __user *user_kdgkb, int perm)
 		kbs = vt_kdskbsent(kbs, kb_func);
 		spin_unlock_irqrestore(&func_buf_lock, flags);
 
-		ret = 0;
-		break;
+		return 0;
 	}
 
-	kfree(kbs);
-
-	return ret;
+	return 0;
 }
 
 int vt_do_kdskled(unsigned int console, int cmd, unsigned long arg, int perm)
-- 
2.51.1

[PATCH 07/10] tty: vt/keyboard: simplify returns from vt_do_kbkeycode_ioctl()

[Jiri Slaby (SUSE)]

Return immediately when something goes wrong in vt_do_kbkeycode_ioctl().
This makes the code flow more obvious.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
 drivers/tty/vt/keyboard.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/tty/vt/keyboard.c b/drivers/tty/vt/keyboard.c
index 65913a137862..487518a696e6 100644
--- a/drivers/tty/vt/keyboard.c
+++ b/drivers/tty/vt/keyboard.c
@@ -1879,27 +1879,27 @@ int vt_do_kdskbmeta(unsigned int console, unsigned int arg)
 	return ret;
 }
 
-int vt_do_kbkeycode_ioctl(int cmd, struct kbkeycode __user *user_kbkc,
-								int perm)
+int vt_do_kbkeycode_ioctl(int cmd, struct kbkeycode __user *user_kbkc, int perm)
 {
 	struct kbkeycode tmp;
-	int kc = 0;
+	int kc;
 
 	if (copy_from_user(&tmp, user_kbkc, sizeof(struct kbkeycode)))
 		return -EFAULT;
+
 	switch (cmd) {
 	case KDGETKEYCODE:
 		kc = getkeycode(tmp.scancode);
-		if (kc >= 0)
-			kc = put_user(kc, &user_kbkc->keycode);
-		break;
+		if (kc < 0)
+			return kc;
+		return put_user(kc, &user_kbkc->keycode);
 	case KDSETKEYCODE:
 		if (!perm)
 			return -EPERM;
-		kc = setkeycode(tmp.scancode, tmp.keycode);
-		break;
+		return setkeycode(tmp.scancode, tmp.keycode);
 	}
-	return kc;
+
+	return 0;
 }
 
 static unsigned short vt_kdgkbent(unsigned char kbdmode, unsigned char idx,
-- 
2.51.1

[PATCH 08/10] tty: vt/keyboard: use guard()s

[Jiri Slaby (SUSE)]

Use guards in the vt/keyboard code. This improves readability, makes
error handling easier, and marks locked portions of code explicit. All
that while being sure the lock is unlocked.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
 drivers/tty/vt/keyboard.c | 210 ++++++++++++++------------------------
 1 file changed, 74 insertions(+), 136 deletions(-)

diff --git a/drivers/tty/vt/keyboard.c b/drivers/tty/vt/keyboard.c
index 487518a696e6..d65fc60dd7be 100644
--- a/drivers/tty/vt/keyboard.c
+++ b/drivers/tty/vt/keyboard.c
@@ -424,8 +424,6 @@ static void do_compute_shiftstate(void)
 /* We still have to export this method to vt.c */
 void vt_set_leds_compute_shiftstate(void)
 {
-	unsigned long flags;
-
 	/*
 	 * When VT is switched, the keyboard led needs to be set once.
 	 * Ensure that after the switch is completed, the state of the
@@ -434,9 +432,8 @@ void vt_set_leds_compute_shiftstate(void)
 	vt_switch = true;
 	set_leds();
 
-	spin_lock_irqsave(&kbd_event_lock, flags);
+	guard(spinlock_irqsave)(&kbd_event_lock);
 	do_compute_shiftstate();
-	spin_unlock_irqrestore(&kbd_event_lock, flags);
 }
 
 /*
@@ -625,13 +622,12 @@ static void fn_compose(struct vc_data *vc)
 
 static void fn_spawn_con(struct vc_data *vc)
 {
-	spin_lock(&vt_spawn_con.lock);
+	guard(spinlock)(&vt_spawn_con.lock);
 	if (vt_spawn_con.pid)
 		if (kill_pid(vt_spawn_con.pid, vt_spawn_con.sig, 1)) {
 			put_pid(vt_spawn_con.pid);
 			vt_spawn_con.pid = NULL;
 		}
-	spin_unlock(&vt_spawn_con.lock);
 }
 
 static void fn_SAK(struct vc_data *vc)
@@ -762,13 +758,9 @@ static void k_fn(struct vc_data *vc, unsigned char value, char up_flag)
 		return;
 
 	if ((unsigned)value < ARRAY_SIZE(func_table)) {
-		unsigned long flags;
-
-		spin_lock_irqsave(&func_buf_lock, flags);
+		guard(spinlock_irqsave)(&func_buf_lock);
 		if (func_table[value])
 			puts_queue(vc, func_table[value]);
-		spin_unlock_irqrestore(&func_buf_lock, flags);
-
 	} else
 		pr_err("k_fn called with value=%d\n", value);
 }
@@ -1140,8 +1132,7 @@ static unsigned char getledstate(void)
 
 void setledstate(struct kbd_struct *kb, unsigned int led)
 {
-        unsigned long flags;
-        spin_lock_irqsave(&led_lock, flags);
+	guard(spinlock_irqsave)(&led_lock);
 	if (!(led & ~7)) {
 		ledioctl = led;
 		kb->ledmode = LED_SHOW_IOCTL;
@@ -1149,7 +1140,6 @@ void setledstate(struct kbd_struct *kb, unsigned int led)
 		kb->ledmode = LED_SHOW_FLAGS;
 
 	set_leds();
-	spin_unlock_irqrestore(&led_lock, flags);
 }
 
 static inline unsigned char getleds(void)
@@ -1172,14 +1162,9 @@ static inline unsigned char getleds(void)
 int vt_get_leds(unsigned int console, int flag)
 {
 	struct kbd_struct *kb = &kbd_table[console];
-	int ret;
-	unsigned long flags;
 
-	spin_lock_irqsave(&led_lock, flags);
-	ret = vc_kbd_led(kb, flag);
-	spin_unlock_irqrestore(&led_lock, flags);
-
-	return ret;
+	guard(spinlock_irqsave)(&led_lock);
+	return vc_kbd_led(kb, flag);
 }
 EXPORT_SYMBOL_GPL(vt_get_leds);
 
@@ -1213,11 +1198,10 @@ void vt_set_led_state(unsigned int console, int leds)
 void vt_kbd_con_start(unsigned int console)
 {
 	struct kbd_struct *kb = &kbd_table[console];
-	unsigned long flags;
-	spin_lock_irqsave(&led_lock, flags);
+
+	guard(spinlock_irqsave)(&led_lock);
 	clr_vc_kbd_led(kb, VC_SCROLLOCK);
 	set_leds();
-	spin_unlock_irqrestore(&led_lock, flags);
 }
 
 /**
@@ -1230,11 +1214,10 @@ void vt_kbd_con_start(unsigned int console)
 void vt_kbd_con_stop(unsigned int console)
 {
 	struct kbd_struct *kb = &kbd_table[console];
-	unsigned long flags;
-	spin_lock_irqsave(&led_lock, flags);
+
+	guard(spinlock_irqsave)(&led_lock);
 	set_vc_kbd_led(kb, VC_SCROLLOCK);
 	set_leds();
-	spin_unlock_irqrestore(&led_lock, flags);
 }
 
 /*
@@ -1246,12 +1229,11 @@ void vt_kbd_con_stop(unsigned int console)
 static void kbd_bh(struct tasklet_struct *unused)
 {
 	unsigned int leds;
-	unsigned long flags;
 
-	spin_lock_irqsave(&led_lock, flags);
-	leds = getleds();
-	leds |= (unsigned int)kbd->lockstate << 8;
-	spin_unlock_irqrestore(&led_lock, flags);
+	scoped_guard(spinlock_irqsave, &led_lock) {
+		leds = getleds();
+		leds |= (unsigned int)kbd->lockstate << 8;
+	}
 
 	if (vt_switch) {
 		ledstate = ~leds;
@@ -1525,15 +1507,13 @@ static void kbd_event(struct input_handle *handle, unsigned int event_type,
 		      unsigned int event_code, int value)
 {
 	/* We are called with interrupts disabled, just take the lock */
-	spin_lock(&kbd_event_lock);
-
-	if (event_type == EV_MSC && event_code == MSC_RAW &&
-			kbd_is_hw_raw(handle->dev))
-		kbd_rawcode(value);
-	if (event_type == EV_KEY && event_code <= KEY_MAX)
-		kbd_keycode(event_code, value, kbd_is_hw_raw(handle->dev));
-
-	spin_unlock(&kbd_event_lock);
+	scoped_guard(spinlock, &kbd_event_lock) {
+		if (event_type == EV_MSC && event_code == MSC_RAW &&
+				kbd_is_hw_raw(handle->dev))
+			kbd_rawcode(value);
+		if (event_type == EV_KEY && event_code <= KEY_MAX)
+			kbd_keycode(event_code, value, kbd_is_hw_raw(handle->dev));
+	}
 
 	tasklet_schedule(&keyboard_tasklet);
 	do_poke_blanked_console = 1;
@@ -1680,7 +1660,6 @@ int __init kbd_init(void)
  */
 int vt_do_diacrit(unsigned int cmd, void __user *udp, int perm)
 {
-	unsigned long flags;
 	int asize;
 
 	switch (cmd) {
@@ -1696,18 +1675,14 @@ int vt_do_diacrit(unsigned int cmd, void __user *udp, int perm)
 
 		/* Lock the diacriticals table, make a copy and then
 		   copy it after we unlock */
-		spin_lock_irqsave(&kbd_event_lock, flags);
-
-		asize = accent_table_size;
-		for (i = 0; i < asize; i++) {
-			dia[i].diacr = conv_uni_to_8bit(
-						accent_table[i].diacr);
-			dia[i].base = conv_uni_to_8bit(
-						accent_table[i].base);
-			dia[i].result = conv_uni_to_8bit(
-						accent_table[i].result);
+		scoped_guard(spinlock_irqsave, &kbd_event_lock) {
+			asize = accent_table_size;
+			for (i = 0; i < asize; i++) {
+				dia[i].diacr = conv_uni_to_8bit(accent_table[i].diacr);
+				dia[i].base = conv_uni_to_8bit(accent_table[i].base);
+				dia[i].result = conv_uni_to_8bit(accent_table[i].result);
+			}
 		}
-		spin_unlock_irqrestore(&kbd_event_lock, flags);
 
 		if (put_user(asize, &a->kb_cnt))
 			return -EFAULT;
@@ -1726,12 +1701,10 @@ int vt_do_diacrit(unsigned int cmd, void __user *udp, int perm)
 
 		/* Lock the diacriticals table, make a copy and then
 		   copy it after we unlock */
-		spin_lock_irqsave(&kbd_event_lock, flags);
-
-		asize = accent_table_size;
-		memcpy(buf, accent_table, asize * sizeof(struct kbdiacruc));
-
-		spin_unlock_irqrestore(&kbd_event_lock, flags);
+		scoped_guard(spinlock_irqsave, &kbd_event_lock) {
+			asize = accent_table_size;
+			memcpy(buf, accent_table, asize * sizeof(struct kbdiacruc));
+		}
 
 		if (put_user(asize, &a->kb_cnt))
 			return -EFAULT;
@@ -1762,7 +1735,7 @@ int vt_do_diacrit(unsigned int cmd, void __user *udp, int perm)
 				return PTR_ERR(dia);
 		}
 
-		spin_lock_irqsave(&kbd_event_lock, flags);
+		guard(spinlock_irqsave)(&kbd_event_lock);
 		accent_table_size = ct;
 		for (i = 0; i < ct; i++) {
 			accent_table[i].diacr =
@@ -1772,7 +1745,6 @@ int vt_do_diacrit(unsigned int cmd, void __user *udp, int perm)
 			accent_table[i].result =
 					conv_8bit_to_uni(dia[i].result);
 		}
-		spin_unlock_irqrestore(&kbd_event_lock, flags);
 
 		return 0;
 	}
@@ -1797,13 +1769,12 @@ int vt_do_diacrit(unsigned int cmd, void __user *udp, int perm)
 						ct, sizeof(struct kbdiacruc));
 			if (IS_ERR(buf))
 				return PTR_ERR(buf);
-		} 
-		spin_lock_irqsave(&kbd_event_lock, flags);
+		}
+		guard(spinlock_irqsave)(&kbd_event_lock);
 		if (ct)
 			memcpy(accent_table, buf,
 					ct * sizeof(struct kbdiacruc));
 		accent_table_size = ct;
-		spin_unlock_irqrestore(&kbd_event_lock, flags);
 		return 0;
 	}
 	}
@@ -1821,33 +1792,29 @@ int vt_do_diacrit(unsigned int cmd, void __user *udp, int perm)
 int vt_do_kdskbmode(unsigned int console, unsigned int arg)
 {
 	struct kbd_struct *kb = &kbd_table[console];
-	int ret = 0;
-	unsigned long flags;
 
-	spin_lock_irqsave(&kbd_event_lock, flags);
+	guard(spinlock_irqsave)(&kbd_event_lock);
 	switch(arg) {
 	case K_RAW:
 		kb->kbdmode = VC_RAW;
-		break;
+		return 0;
 	case K_MEDIUMRAW:
 		kb->kbdmode = VC_MEDIUMRAW;
-		break;
+		return 0;
 	case K_XLATE:
 		kb->kbdmode = VC_XLATE;
 		do_compute_shiftstate();
-		break;
+		return 0;
 	case K_UNICODE:
 		kb->kbdmode = VC_UNICODE;
 		do_compute_shiftstate();
-		break;
+		return 0;
 	case K_OFF:
 		kb->kbdmode = VC_OFF;
-		break;
+		return 0;
 	default:
-		ret = -EINVAL;
+		return -EINVAL;
 	}
-	spin_unlock_irqrestore(&kbd_event_lock, flags);
-	return ret;
 }
 
 /**
@@ -1861,22 +1828,18 @@ int vt_do_kdskbmode(unsigned int console, unsigned int arg)
 int vt_do_kdskbmeta(unsigned int console, unsigned int arg)
 {
 	struct kbd_struct *kb = &kbd_table[console];
-	int ret = 0;
-	unsigned long flags;
 
-	spin_lock_irqsave(&kbd_event_lock, flags);
+	guard(spinlock_irqsave)(&kbd_event_lock);
 	switch(arg) {
 	case K_METABIT:
 		clr_vc_kbd_mode(kb, VC_META);
-		break;
+		return 0;
 	case K_ESCPREFIX:
 		set_vc_kbd_mode(kb, VC_META);
-		break;
+		return 0;
 	default:
-		ret = -EINVAL;
+		return -EINVAL;
 	}
-	spin_unlock_irqrestore(&kbd_event_lock, flags);
-	return ret;
 }
 
 int vt_do_kbkeycode_ioctl(int cmd, struct kbkeycode __user *user_kbkc, int perm)
@@ -1905,31 +1868,28 @@ int vt_do_kbkeycode_ioctl(int cmd, struct kbkeycode __user *user_kbkc, int perm)
 static unsigned short vt_kdgkbent(unsigned char kbdmode, unsigned char idx,
 		unsigned char map)
 {
-	unsigned short *key_map, val;
-	unsigned long flags;
+	unsigned short *key_map;
 
 	/* Ensure another thread doesn't free it under us */
-	spin_lock_irqsave(&kbd_event_lock, flags);
+	guard(spinlock_irqsave)(&kbd_event_lock);
 	key_map = key_maps[map];
 	if (key_map) {
-		val = U(key_map[idx]);
+		unsigned short val = U(key_map[idx]);
 		if (kbdmode != VC_UNICODE && KTYP(val) >= NR_TYPES)
-			val = K_HOLE;
-	} else
-		val = idx ? K_HOLE : K_NOSUCHMAP;
-	spin_unlock_irqrestore(&kbd_event_lock, flags);
+			return K_HOLE;
+		return val;
+	}
 
-	return val;
+	return idx ? K_HOLE : K_NOSUCHMAP;
 }
 
 static int vt_kdskbent(unsigned char kbdmode, unsigned char idx,
 		unsigned char map, unsigned short val)
 {
-	unsigned long flags;
 	unsigned short *key_map, oldval;
 
 	if (!idx && val == K_NOSUCHMAP) {
-		spin_lock_irqsave(&kbd_event_lock, flags);
+		guard(spinlock_irqsave)(&kbd_event_lock);
 		/* deallocate map */
 		key_map = key_maps[map];
 		if (map && key_map) {
@@ -1939,7 +1899,6 @@ static int vt_kdskbent(unsigned char kbdmode, unsigned char idx,
 				keymap_count--;
 			}
 		}
-		spin_unlock_irqrestore(&kbd_event_lock, flags);
 
 		return 0;
 	}
@@ -1961,16 +1920,14 @@ static int vt_kdskbent(unsigned char kbdmode, unsigned char idx,
 	if (!new_map)
 		return -ENOMEM;
 
-	spin_lock_irqsave(&kbd_event_lock, flags);
+	guard(spinlock_irqsave)(&kbd_event_lock);
 	key_map = key_maps[map];
 	if (key_map == NULL) {
 		int j;
 
-		if (keymap_count >= MAX_NR_OF_USER_KEYMAPS &&
-		    !capable(CAP_SYS_RESOURCE)) {
-			spin_unlock_irqrestore(&kbd_event_lock, flags);
+		if (keymap_count >= MAX_NR_OF_USER_KEYMAPS && !capable(CAP_SYS_RESOURCE))
 			return -EPERM;
-		}
+
 		key_map = key_maps[map] = no_free_ptr(new_map);
 		key_map[0] = U(K_ALLOCATED);
 		for (j = 1; j < NR_KEYS; j++)
@@ -1980,19 +1937,15 @@ static int vt_kdskbent(unsigned char kbdmode, unsigned char idx,
 
 	oldval = U(key_map[idx]);
 	if (val == oldval)
-		goto out;
+		return 0;
 
 	/* Attention Key */
-	if ((oldval == K_SAK || val == K_SAK) && !capable(CAP_SYS_ADMIN)) {
-		spin_unlock_irqrestore(&kbd_event_lock, flags);
+	if ((oldval == K_SAK || val == K_SAK) && !capable(CAP_SYS_ADMIN))
 		return -EPERM;
-	}
 
 	key_map[idx] = U(val);
 	if (!map && (KTYP(oldval) == KT_SHIFT || KTYP(val) == KT_SHIFT))
 		do_compute_shiftstate();
-out:
-	spin_unlock_irqrestore(&kbd_event_lock, flags);
 
 	return 0;
 }
@@ -2038,7 +1991,6 @@ static char *vt_kdskbsent(char *kbs, unsigned char cur)
 int vt_do_kdgkb_ioctl(int cmd, struct kbsentry __user *user_kdgkb, int perm)
 {
 	unsigned char kb_func;
-	unsigned long flags;
 
 	if (get_user(kb_func, &user_kdgkb->kb_func))
 		return -EFAULT;
@@ -2054,9 +2006,8 @@ int vt_do_kdgkb_ioctl(int cmd, struct kbsentry __user *user_kdgkb, int perm)
 		if (!kbs)
 			return -ENOMEM;
 
-		spin_lock_irqsave(&func_buf_lock, flags);
-		len = strscpy(kbs, func_table[kb_func] ? : "", len);
-		spin_unlock_irqrestore(&func_buf_lock, flags);
+		scoped_guard(spinlock_irqsave, &func_buf_lock)
+			len = strscpy(kbs, func_table[kb_func] ? : "", len);
 
 		if (len < 0)
 			return -ENOSPC;
@@ -2075,9 +2026,8 @@ int vt_do_kdgkb_ioctl(int cmd, struct kbsentry __user *user_kdgkb, int perm)
 		if (IS_ERR(kbs))
 			return PTR_ERR(kbs);
 
-		spin_lock_irqsave(&func_buf_lock, flags);
+		guard(spinlock_irqsave)(&func_buf_lock);
 		kbs = vt_kdskbsent(kbs, kb_func);
-		spin_unlock_irqrestore(&func_buf_lock, flags);
 
 		return 0;
 	}
@@ -2088,16 +2038,14 @@ int vt_do_kdgkb_ioctl(int cmd, struct kbsentry __user *user_kdgkb, int perm)
 int vt_do_kdskled(unsigned int console, int cmd, unsigned long arg, int perm)
 {
 	struct kbd_struct *kb = &kbd_table[console];
-        unsigned long flags;
 	unsigned char ucval;
 
         switch(cmd) {
 	/* the ioctls below read/set the flags usually shown in the leds */
 	/* don't use them - they will go away without warning */
 	case KDGKBLED:
-                spin_lock_irqsave(&kbd_event_lock, flags);
-		ucval = kb->ledflagstate | (kb->default_ledflagstate << 4);
-                spin_unlock_irqrestore(&kbd_event_lock, flags);
+		scoped_guard(spinlock_irqsave, &kbd_event_lock)
+			ucval = kb->ledflagstate | (kb->default_ledflagstate << 4);
 		return put_user(ucval, (char __user *)arg);
 
 	case KDSKBLED:
@@ -2105,11 +2053,11 @@ int vt_do_kdskled(unsigned int console, int cmd, unsigned long arg, int perm)
 			return -EPERM;
 		if (arg & ~0x77)
 			return -EINVAL;
-                spin_lock_irqsave(&led_lock, flags);
-		kb->ledflagstate = (arg & 7);
-		kb->default_ledflagstate = ((arg >> 4) & 7);
-		set_leds();
-                spin_unlock_irqrestore(&led_lock, flags);
+		scoped_guard(spinlock_irqsave, &led_lock) {
+			kb->ledflagstate = (arg & 7);
+			kb->default_ledflagstate = ((arg >> 4) & 7);
+			set_leds();
+		}
 		return 0;
 
 	/* the ioctls below only set the lights, not the functions */
@@ -2166,11 +2114,8 @@ int vt_do_kdgkbmeta(unsigned int console)
  */
 void vt_reset_unicode(unsigned int console)
 {
-	unsigned long flags;
-
-	spin_lock_irqsave(&kbd_event_lock, flags);
+	guard(spinlock_irqsave)(&kbd_event_lock);
 	kbd_table[console].kbdmode = default_utf8 ? VC_UNICODE : VC_XLATE;
-	spin_unlock_irqrestore(&kbd_event_lock, flags);
 }
 
 /**
@@ -2195,22 +2140,19 @@ int vt_get_shift_state(void)
 void vt_reset_keyboard(unsigned int console)
 {
 	struct kbd_struct *kb = &kbd_table[console];
-	unsigned long flags;
 
-	spin_lock_irqsave(&kbd_event_lock, flags);
+	guard(spinlock_irqsave)(&kbd_event_lock);
 	set_vc_kbd_mode(kb, VC_REPEAT);
 	clr_vc_kbd_mode(kb, VC_CKMODE);
 	clr_vc_kbd_mode(kb, VC_APPLIC);
 	clr_vc_kbd_mode(kb, VC_CRLF);
 	kb->lockstate = 0;
 	kb->slockstate = 0;
-	spin_lock(&led_lock);
+	guard(spinlock)(&led_lock);
 	kb->ledmode = LED_SHOW_FLAGS;
 	kb->ledflagstate = kb->default_ledflagstate;
-	spin_unlock(&led_lock);
 	/* do not do set_leds here because this causes an endless tasklet loop
 	   when the keyboard hasn't been initialized yet */
-	spin_unlock_irqrestore(&kbd_event_lock, flags);
 }
 
 /**
@@ -2240,11 +2182,9 @@ int vt_get_kbd_mode_bit(unsigned int console, int bit)
 void vt_set_kbd_mode_bit(unsigned int console, int bit)
 {
 	struct kbd_struct *kb = &kbd_table[console];
-	unsigned long flags;
 
-	spin_lock_irqsave(&kbd_event_lock, flags);
+	guard(spinlock_irqsave)(&kbd_event_lock);
 	set_vc_kbd_mode(kb, bit);
-	spin_unlock_irqrestore(&kbd_event_lock, flags);
 }
 
 /**
@@ -2259,9 +2199,7 @@ void vt_set_kbd_mode_bit(unsigned int console, int bit)
 void vt_clr_kbd_mode_bit(unsigned int console, int bit)
 {
 	struct kbd_struct *kb = &kbd_table[console];
-	unsigned long flags;
 
-	spin_lock_irqsave(&kbd_event_lock, flags);
+	guard(spinlock_irqsave)(&kbd_event_lock);
 	clr_vc_kbd_mode(kb, bit);
-	spin_unlock_irqrestore(&kbd_event_lock, flags);
 }
-- 
2.51.1

[PATCH 09/10] serial: serial_core: simplify uart_ioctl() returns

[Jiri Slaby (SUSE)]

Neither uart_do_autoconfig(), nor uart_wait_modem_status() can return
-ENOIOCTLCMD. The ENOIOCTLCMD checks are there to check if 'cmd' matched
against TIOCSERCONFIG, and TIOCMIWAIT respectively. (With 0 or error in
'ret', it does not matter.)

Therefore, the code can simply return from the TIOCSERCONFIG and
TIOCMIWAIT spots immediately.

To be more explicit, use 'if' instead of switch-case for those single
values.

And return without jumping to the 'out' label -- it can be removed too.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
 drivers/tty/serial/serial_core.c | 35 ++++++++------------------------
 1 file changed, 9 insertions(+), 26 deletions(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 4757293ece8c..74018fb8a4e7 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -1560,37 +1560,20 @@ uart_ioctl(struct tty_struct *tty, unsigned int cmd, unsigned long arg)
 	void __user *uarg = (void __user *)arg;
 	int ret = -ENOIOCTLCMD;
 
-
-	/*
-	 * These ioctls don't rely on the hardware to be present.
-	 */
-	switch (cmd) {
-	case TIOCSERCONFIG:
+	/* This ioctl doesn't rely on the hardware to be present. */
+	if (cmd == TIOCSERCONFIG) {
 		down_write(&tty->termios_rwsem);
 		ret = uart_do_autoconfig(tty, state);
 		up_write(&tty->termios_rwsem);
-		break;
-	}
-
-	if (ret != -ENOIOCTLCMD)
-		goto out;
-
-	if (tty_io_error(tty)) {
-		ret = -EIO;
-		goto out;
+		return ret;
 	}
 
-	/*
-	 * The following should only be used when hardware is present.
-	 */
-	switch (cmd) {
-	case TIOCMIWAIT:
-		ret = uart_wait_modem_status(state, arg);
-		break;
-	}
+	if (tty_io_error(tty))
+		return -EIO;
 
-	if (ret != -ENOIOCTLCMD)
-		goto out;
+	/* This should only be used when the hardware is present. */
+	if (cmd == TIOCMIWAIT)
+		return uart_wait_modem_status(state, arg);
 
 	/* rs485_config requires more locking than others */
 	if (cmd == TIOCSRS485)
@@ -1638,7 +1621,7 @@ uart_ioctl(struct tty_struct *tty, unsigned int cmd, unsigned long arg)
 	mutex_unlock(&port->mutex);
 	if (cmd == TIOCSRS485)
 		up_write(&tty->termios_rwsem);
-out:
+
 	return ret;
 }
 
-- 
2.51.1

[PATCH 10/10] serial: serial_core: use guard()s

[Jiri Slaby (SUSE)]

Use guards in the serial_core code. This improves readability, makes
error handling easier, and marks locked portions of code explicit. All
that while being sure the lock is unlocked.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
 drivers/tty/serial/serial_core.c | 127 +++++++++++++------------------
 1 file changed, 55 insertions(+), 72 deletions(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 74018fb8a4e7..c532235f8d55 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -1034,9 +1034,8 @@ static int uart_set_info_user(struct tty_struct *tty, struct serial_struct *ss)
 {
 	struct uart_state *state = tty->driver_data;
 	struct tty_port *port = &state->port;
-	int retval;
 
-	down_write(&tty->termios_rwsem);
+	guard(rwsem_write)(&tty->termios_rwsem);
 	/*
 	 * This semaphore protects port->count.  It is also
 	 * very useful to prevent opens.  Also, take the
@@ -1044,11 +1043,8 @@ static int uart_set_info_user(struct tty_struct *tty, struct serial_struct *ss)
 	 * module insertion/removal doesn't change anything
 	 * under us.
 	 */
-	mutex_lock(&port->mutex);
-	retval = uart_set_info(tty, port, state, ss);
-	mutex_unlock(&port->mutex);
-	up_write(&tty->termios_rwsem);
-	return retval;
+	guard(mutex)(&port->mutex);
+	return uart_set_info(tty, port, state, ss);
 }
 
 /**
@@ -1562,10 +1558,8 @@ uart_ioctl(struct tty_struct *tty, unsigned int cmd, unsigned long arg)
 
 	/* This ioctl doesn't rely on the hardware to be present. */
 	if (cmd == TIOCSERCONFIG) {
-		down_write(&tty->termios_rwsem);
-		ret = uart_do_autoconfig(tty, state);
-		up_write(&tty->termios_rwsem);
-		return ret;
+		guard(rwsem_write)(&tty->termios_rwsem);
+		return uart_do_autoconfig(tty, state);
 	}
 
 	if (tty_io_error(tty))
@@ -1579,46 +1573,46 @@ uart_ioctl(struct tty_struct *tty, unsigned int cmd, unsigned long arg)
 	if (cmd == TIOCSRS485)
 		down_write(&tty->termios_rwsem);
 
-	mutex_lock(&port->mutex);
-	uport = uart_port_check(state);
+	scoped_guard(mutex, &port->mutex) {
+		uport = uart_port_check(state);
 
-	if (!uport || tty_io_error(tty)) {
-		ret = -EIO;
-		goto out_up;
-	}
+		if (!uport || tty_io_error(tty)) {
+			ret = -EIO;
+			break;
+		}
 
-	/*
-	 * All these rely on hardware being present and need to be
-	 * protected against the tty being hung up.
-	 */
+		/*
+		 * All these rely on hardware being present and need to be
+		 * protected against the tty being hung up.
+		 */
 
-	switch (cmd) {
-	case TIOCSERGETLSR: /* Get line status register */
-		ret = uart_get_lsr_info(tty, state, uarg);
-		break;
+		switch (cmd) {
+		case TIOCSERGETLSR: /* Get line status register */
+			ret = uart_get_lsr_info(tty, state, uarg);
+			break;
 
-	case TIOCGRS485:
-		ret = uart_get_rs485_config(uport, uarg);
-		break;
+		case TIOCGRS485:
+			ret = uart_get_rs485_config(uport, uarg);
+			break;
 
-	case TIOCSRS485:
-		ret = uart_set_rs485_config(tty, uport, uarg);
-		break;
+		case TIOCSRS485:
+			ret = uart_set_rs485_config(tty, uport, uarg);
+			break;
 
-	case TIOCSISO7816:
-		ret = uart_set_iso7816_config(state->uart_port, uarg);
-		break;
+		case TIOCSISO7816:
+			ret = uart_set_iso7816_config(state->uart_port, uarg);
+			break;
 
-	case TIOCGISO7816:
-		ret = uart_get_iso7816_config(state->uart_port, uarg);
-		break;
-	default:
-		if (uport->ops->ioctl)
-			ret = uport->ops->ioctl(uport, cmd, arg);
-		break;
+		case TIOCGISO7816:
+			ret = uart_get_iso7816_config(state->uart_port, uarg);
+			break;
+		default:
+			if (uport->ops->ioctl)
+				ret = uport->ops->ioctl(uport, cmd, arg);
+			break;
+		}
 	}
-out_up:
-	mutex_unlock(&port->mutex);
+
 	if (cmd == TIOCSRS485)
 		up_write(&tty->termios_rwsem);
 
@@ -1634,11 +1628,10 @@ static void uart_set_ldisc(struct tty_struct *tty)
 	if (!tty_port_initialized(port))
 		return;
 
-	mutex_lock(&state->port.mutex);
+	guard(mutex)(&state->port.mutex);
 	uport = uart_port_check(state);
 	if (uport && uport->ops->set_ldisc)
 		uport->ops->set_ldisc(uport, &tty->termios);
-	mutex_unlock(&state->port.mutex);
 }
 
 static void uart_set_termios(struct tty_struct *tty,
@@ -1712,9 +1705,8 @@ static void uart_close(struct tty_struct *tty, struct file *filp)
 
 		state = drv->state + tty->index;
 		port = &state->port;
-		spin_lock_irq(&port->lock);
+		guard(spinlock_irq)(&port->lock);
 		--port->count;
-		spin_unlock_irq(&port->lock);
 		return;
 	}
 
@@ -1826,20 +1818,18 @@ static void uart_hangup(struct tty_struct *tty)
 	struct uart_state *state = tty->driver_data;
 	struct tty_port *port = &state->port;
 	struct uart_port *uport;
-	unsigned long flags;
 
 	pr_debug("uart_hangup(%d)\n", tty->index);
 
-	mutex_lock(&port->mutex);
+	guard(mutex)(&port->mutex);
 	uport = uart_port_check(state);
 	WARN(!uport, "hangup of detached port!\n");
 
 	if (tty_port_active(port)) {
 		uart_flush_buffer(tty);
 		uart_shutdown(tty, state);
-		spin_lock_irqsave(&port->lock, flags);
-		port->count = 0;
-		spin_unlock_irqrestore(&port->lock, flags);
+		scoped_guard(spinlock_irqsave, &port->lock)
+			port->count = 0;
 		tty_port_set_active(port, false);
 		tty_port_tty_set(port, NULL);
 		if (uport && !uart_console(uport))
@@ -1847,7 +1837,6 @@ static void uart_hangup(struct tty_struct *tty)
 		wake_up_interruptible(&port->open_wait);
 		wake_up_interruptible(&port->delta_msr_wait);
 	}
-	mutex_unlock(&port->mutex);
 }
 
 /* uport == NULL if uart_port has already been removed */
@@ -2952,11 +2941,11 @@ static ssize_t console_show(struct device *dev,
 	struct uart_port *uport;
 	bool console = false;
 
-	mutex_lock(&port->mutex);
-	uport = uart_port_check(state);
-	if (uport)
-		console = uart_console_registered(uport);
-	mutex_unlock(&port->mutex);
+	scoped_guard(mutex, &port->mutex) {
+		uport = uart_port_check(state);
+		if (uport)
+			console = uart_console_registered(uport);
+	}
 
 	return sprintf(buf, "%c\n", console ? 'Y' : 'N');
 }
@@ -3141,17 +3130,14 @@ static void serial_core_remove_one_port(struct uart_driver *drv,
 	struct tty_port *port = &state->port;
 	struct uart_port *uart_port;
 
-	mutex_lock(&port->mutex);
-	uart_port = uart_port_check(state);
-	if (uart_port != uport)
-		dev_alert(uport->dev, "Removing wrong port: %p != %p\n",
-			  uart_port, uport);
+	scoped_guard(mutex, &port->mutex) {
+		uart_port = uart_port_check(state);
+		if (uart_port != uport)
+			dev_alert(uport->dev, "Removing wrong port: %p != %p\n", uart_port, uport);
 
-	if (!uart_port) {
-		mutex_unlock(&port->mutex);
-		return;
+		if (!uart_port)
+			return;
 	}
-	mutex_unlock(&port->mutex);
 
 	/*
 	 * Remove the devices from the tty layer
@@ -3180,11 +3166,10 @@ static void serial_core_remove_one_port(struct uart_driver *drv,
 	uport->type = PORT_UNKNOWN;
 	uport->port_dev = NULL;
 
-	mutex_lock(&port->mutex);
+	guard(mutex)(&port->mutex);
 	WARN_ON(atomic_dec_return(&state->refcount) < 0);
 	wait_event(state->remove_wait, !atomic_read(&state->refcount));
 	state->uart_port = NULL;
-	mutex_unlock(&port->mutex);
 }
 
 /**
@@ -3337,7 +3322,7 @@ void serial_core_unregister_port(struct uart_driver *drv, struct uart_port *port
 	struct serial_ctrl_device *ctrl_dev = serial_core_get_ctrl_dev(port_dev);
 	int ctrl_id = port->ctrl_id;
 
-	mutex_lock(&port_mutex);
+	guard(mutex)(&port_mutex);
 
 	port->flags |= UPF_DEAD;
 
@@ -3349,8 +3334,6 @@ void serial_core_unregister_port(struct uart_driver *drv, struct uart_port *port
 	/* Drop the serial core controller device if no ports are using it */
 	if (!serial_core_ctrl_find(drv, phys_dev, ctrl_id))
 		serial_base_ctrl_device_remove(ctrl_dev);
-
-	mutex_unlock(&port_mutex);
 }
 
 /**
-- 
2.51.1