Re: [PATCH] n_tty: fix redirected_tty_write checks after write_iter conversion

[Linus Torvalds <torvalds@linux-foundation.org>]

[-- Attachment #1: Type: text/plain, Size: 843 bytes --]

On Mon, Jan 25, 2021 at 11:09 AM Sami Tolvanen <samitolvanen@google.com> wrote:
>
> Commit 9bb48c82aced ("tty: implement write_iter") converted the tty
> layer to use write_iter. Fix the redirected_tty_write declaration
> also in n_tty and change the comparisons to use write_iter instead of
> write.

Duh.

Obvious ACK from me.

The only thing I'd ask is that the declaration for
redirected_tty_write() be moved to a proper header file
(<linux/tty.h>.

Because the reason I didn't notice this was literally that n_tty.c did
its own private 'extern' declaration of that function. Which is
horribly wrong, exactly because it doesn't then ever notice when the
declaration is changed.

So I'd suggest doing the patch as attached - but please keep Sami's
credit, this is purely a "declare the function in the proper place"
fix.

            Linus

[-- Attachment #2: patch --]
[-- Type: application/octet-stream, Size: 2590 bytes --]

 drivers/tty/n_tty.c  | 7 ++-----
 drivers/tty/tty_io.c | 2 --
 include/linux/tty.h  | 1 +
 3 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 319d68c8a5df..219e85756171 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -2081,9 +2081,6 @@ static int canon_copy_from_read_buf(struct tty_struct *tty,
 	return 0;
 }
 
-extern ssize_t redirected_tty_write(struct file *, const char __user *,
-							size_t, loff_t *);
-
 /**
  *	job_control		-	check job control
  *	@tty: tty
@@ -2105,7 +2102,7 @@ static int job_control(struct tty_struct *tty, struct file *file)
 	/* NOTE: not yet done after every sleep pending a thorough
 	   check of the logic of this change. -- jlc */
 	/* don't stop on /dev/console */
-	if (file->f_op->write == redirected_tty_write)
+	if (file->f_op->write_iter == redirected_tty_write)
 		return 0;
 
 	return __tty_check_change(tty, SIGTTIN);
@@ -2309,7 +2306,7 @@ static ssize_t n_tty_write(struct tty_struct *tty, struct file *file,
 	ssize_t retval = 0;
 
 	/* Job control check -- must be done at start (POSIX.1 7.1.1.4). */
-	if (L_TOSTOP(tty) && file->f_op->write != redirected_tty_write) {
+	if (L_TOSTOP(tty) && file->f_op->write_iter != redirected_tty_write) {
 		retval = tty_check_change(tty);
 		if (retval)
 			return retval;
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 4a208a95e921..48de20916ca7 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -144,10 +144,8 @@ DEFINE_MUTEX(tty_mutex);
 
 static ssize_t tty_read(struct file *, char __user *, size_t, loff_t *);
 static ssize_t tty_write(struct kiocb *, struct iov_iter *);
-ssize_t redirected_tty_write(struct kiocb *, struct iov_iter *);
 static __poll_t tty_poll(struct file *, poll_table *);
 static int tty_open(struct inode *, struct file *);
-long tty_ioctl(struct file *file, unsigned int cmd, unsigned long arg);
 #ifdef CONFIG_COMPAT
 static long tty_compat_ioctl(struct file *file, unsigned int cmd,
 				unsigned long arg);
diff --git a/include/linux/tty.h b/include/linux/tty.h
index c873f475f0a7..37803f3e6d49 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -421,6 +421,7 @@ extern void tty_kclose(struct tty_struct *tty);
 extern int tty_dev_name_to_number(const char *name, dev_t *number);
 extern int tty_ldisc_lock(struct tty_struct *tty, unsigned long timeout);
 extern void tty_ldisc_unlock(struct tty_struct *tty);
+extern ssize_t redirected_tty_write(struct kiocb *, struct iov_iter *);
 #else
 static inline void tty_kref_put(struct tty_struct *tty)
 { }