[PATCH 01/24] n_hdlc: remove tracing debug prints

[PATCH 01/24] n_hdlc: remove tracing debug prints

[Jiri Slaby]

We can trace functions using ftrace, so there is no need for this
additional prints. Remove them.

We keep only those which print some additional info, not only function
name & "entry"/"exit".

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/n_hdlc.c | 32 ++------------------------------
 1 file changed, 2 insertions(+), 30 deletions(-)

diff --git a/drivers/tty/n_hdlc.c b/drivers/tty/n_hdlc.c
index 27b506bf03ce..9e115ecf920d 100644
--- a/drivers/tty/n_hdlc.c
+++ b/drivers/tty/n_hdlc.c
@@ -232,10 +232,7 @@ static void n_hdlc_release(struct n_hdlc *n_hdlc)
 {
 	struct tty_struct *tty = n_hdlc2tty (n_hdlc);
 	struct n_hdlc_buf *buf;
-	
-	if (debuglevel >= DEBUG_LEVEL_INFO)	
-		printk("%s(%d)n_hdlc_release() called\n",__FILE__,__LINE__);
-		
+
 	/* Ensure that the n_hdlcd process is not hanging on select()/poll() */
 	wake_up_interruptible (&tty->read_wait);
 	wake_up_interruptible (&tty->write_wait);
@@ -287,9 +284,6 @@ static void n_hdlc_tty_close(struct tty_struct *tty)
 {
 	struct n_hdlc *n_hdlc = tty2n_hdlc (tty);
 
-	if (debuglevel >= DEBUG_LEVEL_INFO)	
-		printk("%s(%d)n_hdlc_tty_close() called\n",__FILE__,__LINE__);
-		
 	if (n_hdlc != NULL) {
 		if (n_hdlc->magic != HDLC_MAGIC) {
 			printk (KERN_WARNING"n_hdlc: trying to close unopened tty!\n");
@@ -309,10 +303,6 @@ static void n_hdlc_tty_close(struct tty_struct *tty)
 			n_hdlc_release (n_hdlc);
 		}
 	}
-	
-	if (debuglevel >= DEBUG_LEVEL_INFO)	
-		printk("%s(%d)n_hdlc_tty_close() success\n",__FILE__,__LINE__);
-		
 }	/* end of n_hdlc_tty_close() */
 
 /**
@@ -353,10 +343,7 @@ static int n_hdlc_tty_open (struct tty_struct *tty)
 	
 	/* flush receive data from driver */
 	tty_driver_flush_buffer(tty);
-		
-	if (debuglevel >= DEBUG_LEVEL_INFO)	
-		printk("%s(%d)n_hdlc_tty_open() success\n",__FILE__,__LINE__);
-		
+
 	return 0;
 	
 }	/* end of n_tty_hdlc_open() */
@@ -376,8 +363,6 @@ static void n_hdlc_send_frames(struct n_hdlc *n_hdlc, struct tty_struct *tty)
 	unsigned long flags;
 	struct n_hdlc_buf *tbuf;
 
-	if (debuglevel >= DEBUG_LEVEL_INFO)	
-		printk("%s(%d)n_hdlc_send_frames() called\n",__FILE__,__LINE__);
  check_again:
 		
  	spin_lock_irqsave(&n_hdlc->tx_buf_list.spinlock, flags);
@@ -447,10 +432,6 @@ static void n_hdlc_send_frames(struct n_hdlc *n_hdlc, struct tty_struct *tty)
 	
         if (n_hdlc->woke_up)
 	  goto check_again;
-
-	if (debuglevel >= DEBUG_LEVEL_INFO)	
-		printk("%s(%d)n_hdlc_send_frames() exit\n",__FILE__,__LINE__);
-		
 }	/* end of n_hdlc_send_frames() */
 
 /**
@@ -463,9 +444,6 @@ static void n_hdlc_tty_wakeup(struct tty_struct *tty)
 {
 	struct n_hdlc *n_hdlc = tty2n_hdlc(tty);
 
-	if (debuglevel >= DEBUG_LEVEL_INFO)	
-		printk("%s(%d)n_hdlc_tty_wakeup() called\n",__FILE__,__LINE__);
-		
 	if (!n_hdlc)
 		return;
 
@@ -564,9 +542,6 @@ static ssize_t n_hdlc_tty_read(struct tty_struct *tty, struct file *file,
 	struct n_hdlc_buf *rbuf;
 	DECLARE_WAITQUEUE(wait, current);
 
-	if (debuglevel >= DEBUG_LEVEL_INFO)	
-		printk("%s(%d)n_hdlc_tty_read() called\n",__FILE__,__LINE__);
-		
 	/* Validate the pointers */
 	if (!n_hdlc)
 		return -EIO;
@@ -802,9 +777,6 @@ static __poll_t n_hdlc_tty_poll(struct tty_struct *tty, struct file *filp,
 	struct n_hdlc *n_hdlc = tty2n_hdlc (tty);
 	__poll_t mask = 0;
 
-	if (debuglevel >= DEBUG_LEVEL_INFO)	
-		printk("%s(%d)n_hdlc_tty_poll() called\n",__FILE__,__LINE__);
-		
 	if (n_hdlc && n_hdlc->magic == HDLC_MAGIC && tty == n_hdlc->tty) {
 		/* queue current process into any wait queue that */
 		/* may awaken in the future (read and write) */
-- 
2.25.0

[PATCH 02/24] n_hdlc: remove unused macros

[Jiri Slaby]

VERSION and bset are unused. Remove them.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/n_hdlc.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/tty/n_hdlc.c b/drivers/tty/n_hdlc.c
index 9e115ecf920d..881da4d440c8 100644
--- a/drivers/tty/n_hdlc.c
+++ b/drivers/tty/n_hdlc.c
@@ -87,9 +87,6 @@
 #include <linux/interrupt.h>
 #include <linux/ptrace.h>
 
-#undef VERSION
-#define VERSION(major,minor,patch) (((((major)<<8)+(minor))<<8)+(patch))
-
 #include <linux/poll.h>
 #include <linux/in.h>
 #include <linux/ioctl.h>
@@ -186,8 +183,6 @@ static void n_hdlc_tty_receive(struct tty_struct *tty, const __u8 *cp,
 			       char *fp, int count);
 static void n_hdlc_tty_wakeup(struct tty_struct *tty);
 
-#define bset(p,b)	((p)[(b) >> 5] |= (1 << ((b) & 0x1f)))
-
 #define tty2n_hdlc(tty)	((struct n_hdlc *) ((tty)->disc_data))
 #define n_hdlc2tty(n_hdlc)	((n_hdlc)->tty)
 
-- 
2.25.0

[PATCH 03/24] n_hdlc: convert debuglevel use to pr_debug

[Jiri Slaby]

With pr_debug we have a fine-grained control about debugging prints. So
convert the use of global debuglevel variable and tests to a commonly
used pr_debug. And drop debuglevel completely.

This also implicitly adds a loglevel to the messages (KERN_DEBUG) as it
was missing on most of them.

And also use __func__ instead of function names explicitly typed.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/n_hdlc.c | 79 +++++++++++++++++---------------------------
 1 file changed, 31 insertions(+), 48 deletions(-)

diff --git a/drivers/tty/n_hdlc.c b/drivers/tty/n_hdlc.c
index 881da4d440c8..e37dab2528a1 100644
--- a/drivers/tty/n_hdlc.c
+++ b/drivers/tty/n_hdlc.c
@@ -160,10 +160,6 @@ static struct n_hdlc_buf *n_hdlc_buf_get(struct n_hdlc_buf_list *list);
 
 static struct n_hdlc *n_hdlc_alloc (void);
 
-/* debug level can be set by insmod for debugging purposes */
-#define DEBUG_LEVEL_INFO	1
-static int debuglevel;
-
 /* max frame size for memory allocations */
 static int maxframe = 4096;
 
@@ -310,11 +306,9 @@ static int n_hdlc_tty_open (struct tty_struct *tty)
 {
 	struct n_hdlc *n_hdlc = tty2n_hdlc (tty);
 
-	if (debuglevel >= DEBUG_LEVEL_INFO)	
-		printk("%s(%d)n_hdlc_tty_open() called (device=%s)\n",
-		__FILE__,__LINE__,
-		tty->name);
-		
+	pr_debug("%s(%d)%s() called (device=%s)\n",
+			__FILE__, __LINE__, __func__, tty->name);
+
 	/* There should not be an existing table for this slot. */
 	if (n_hdlc) {
 		printk (KERN_ERR"n_hdlc_tty_open:tty already associated!\n" );
@@ -372,10 +366,9 @@ static void n_hdlc_send_frames(struct n_hdlc *n_hdlc, struct tty_struct *tty)
 
 	tbuf = n_hdlc_buf_get(&n_hdlc->tx_buf_list);
 	while (tbuf) {
-		if (debuglevel >= DEBUG_LEVEL_INFO)	
-			printk("%s(%d)sending frame %p, count=%d\n",
-				__FILE__,__LINE__,tbuf,tbuf->count);
-			
+		pr_debug("%s(%d)sending frame %p, count=%d\n",
+				__FILE__, __LINE__, tbuf, tbuf->count);
+
 		/* Send the next block of data to device */
 		set_bit(TTY_DO_WRITE_WAKEUP, &tty->flags);
 		actual = tty->ops->write(tty, tbuf->buf, tbuf->count);
@@ -391,10 +384,9 @@ static void n_hdlc_send_frames(struct n_hdlc *n_hdlc, struct tty_struct *tty)
 			actual = tbuf->count;
 		
 		if (actual == tbuf->count) {
-			if (debuglevel >= DEBUG_LEVEL_INFO)	
-				printk("%s(%d)frame %p completed\n",
-					__FILE__,__LINE__,tbuf);
-					
+			pr_debug("%s(%d)frame %p completed\n",
+					__FILE__, __LINE__, tbuf);
+
 			/* free current transmit buffer */
 			n_hdlc_buf_put(&n_hdlc->tx_free_buf_list, tbuf);
 
@@ -404,9 +396,8 @@ static void n_hdlc_send_frames(struct n_hdlc *n_hdlc, struct tty_struct *tty)
 			/* get next pending transmit buffer */
 			tbuf = n_hdlc_buf_get(&n_hdlc->tx_buf_list);
 		} else {
-			if (debuglevel >= DEBUG_LEVEL_INFO)	
-				printk("%s(%d)frame %p pending\n",
-					__FILE__,__LINE__,tbuf);
+			pr_debug("%s(%d)frame %p pending\n",
+					__FILE__, __LINE__, tbuf);
 
 			/*
 			 * the buffer was not accepted by driver,
@@ -467,10 +458,9 @@ static void n_hdlc_tty_receive(struct tty_struct *tty, const __u8 *data,
 	register struct n_hdlc *n_hdlc = tty2n_hdlc (tty);
 	register struct n_hdlc_buf *buf;
 
-	if (debuglevel >= DEBUG_LEVEL_INFO)	
-		printk("%s(%d)n_hdlc_tty_receive() called count=%d\n",
-			__FILE__,__LINE__, count);
-		
+	pr_debug("%s(%d)%s() called count=%d\n",
+			__FILE__, __LINE__, __func__, count);
+
 	/* This can happen if stuff comes in on the backup tty */
 	if (!n_hdlc || tty != n_hdlc->tty)
 		return;
@@ -483,9 +473,8 @@ static void n_hdlc_tty_receive(struct tty_struct *tty, const __u8 *data,
 	}
 	
 	if ( count>maxframe ) {
-		if (debuglevel >= DEBUG_LEVEL_INFO)	
-			printk("%s(%d) rx count>maxframesize, data discarded\n",
-			       __FILE__,__LINE__);
+		pr_debug("%s(%d) rx count>maxframesize, data discarded\n",
+				__FILE__, __LINE__);
 		return;
 	}
 
@@ -500,9 +489,8 @@ static void n_hdlc_tty_receive(struct tty_struct *tty, const __u8 *data,
 	}
 	
 	if (!buf) {
-		if (debuglevel >= DEBUG_LEVEL_INFO)	
-			printk("%s(%d) no more rx buffers, data discarded\n",
-			       __FILE__,__LINE__);
+		pr_debug("%s(%d) no more rx buffers, data discarded\n",
+				__FILE__, __LINE__);
 		return;
 	}
 		
@@ -619,10 +607,9 @@ static ssize_t n_hdlc_tty_write(struct tty_struct *tty, struct file *file,
 	DECLARE_WAITQUEUE(wait, current);
 	struct n_hdlc_buf *tbuf;
 
-	if (debuglevel >= DEBUG_LEVEL_INFO)	
-		printk("%s(%d)n_hdlc_tty_write() called count=%zd\n",
-			__FILE__,__LINE__,count);
-		
+	pr_debug("%s(%d)%s() called count=%zd\n", __FILE__, __LINE__, __func__,
+			count);
+
 	/* Verify pointers */
 	if (!n_hdlc)
 		return -EIO;
@@ -632,11 +619,8 @@ static ssize_t n_hdlc_tty_write(struct tty_struct *tty, struct file *file,
 
 	/* verify frame size */
 	if (count > maxframe ) {
-		if (debuglevel & DEBUG_LEVEL_INFO)
-			printk (KERN_WARNING
-				"n_hdlc_tty_write: truncating user packet "
-				"from %lu to %d\n", (unsigned long) count,
-				maxframe );
+		pr_debug("%s: truncating user packet from %zu to %d\n",
+				__func__, count, maxframe);
 		count = maxframe;
 	}
 	
@@ -704,10 +688,8 @@ static int n_hdlc_tty_ioctl(struct tty_struct *tty, struct file *file,
 	unsigned long flags;
 	struct n_hdlc_buf *buf = NULL;
 
-	if (debuglevel >= DEBUG_LEVEL_INFO)	
-		printk("%s(%d)n_hdlc_tty_ioctl() called %d\n",
-			__FILE__,__LINE__,cmd);
-		
+	pr_debug("%s(%d)%s() called %d\n", __FILE__, __LINE__, __func__, cmd);
+
 	/* Verify the status of the device */
 	if (!n_hdlc || n_hdlc->magic != HDLC_MAGIC)
 		return -EBADF;
@@ -822,8 +804,9 @@ static struct n_hdlc *n_hdlc_alloc(void)
 		buf = kmalloc(struct_size(buf, buf, maxframe), GFP_KERNEL);
 		if (buf)
 			n_hdlc_buf_put(&n_hdlc->rx_free_buf_list,buf);
-		else if (debuglevel >= DEBUG_LEVEL_INFO)	
-			printk("%s(%d)n_hdlc_alloc(), kalloc() failed for rx buffer %d\n",__FILE__,__LINE__, i);
+		else
+			pr_debug("%s(%d)%s(), kmalloc() failed for rx buffer %d\n",
+					__FILE__, __LINE__, __func__, i);
 	}
 	
 	/* allocate free tx buffer list */
@@ -831,8 +814,9 @@ static struct n_hdlc *n_hdlc_alloc(void)
 		buf = kmalloc(struct_size(buf, buf, maxframe), GFP_KERNEL);
 		if (buf)
 			n_hdlc_buf_put(&n_hdlc->tx_free_buf_list,buf);
-		else if (debuglevel >= DEBUG_LEVEL_INFO)	
-			printk("%s(%d)n_hdlc_alloc(), kalloc() failed for tx buffer %d\n",__FILE__,__LINE__, i);
+		else
+			pr_debug("%s(%d)%s(), kmalloc() failed for tx buffer %d\n",
+					__FILE__, __LINE__, __func__, i);
 	}
 	
 	/* Initialize the control block */
@@ -960,6 +944,5 @@ module_exit(n_hdlc_exit);
 
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Paul Fulghum paulkf@microgate.com");
-module_param(debuglevel, int, 0);
 module_param(maxframe, int, 0);
 MODULE_ALIAS_LDISC(N_HDLC);
-- 
2.25.0

[PATCH 04/24] n_hdlc: put init/exit strings directly to prints

[Jiri Slaby]

These strings were put aside from prints to save some bytes after module
load or when built-in -- they were freed after module load (__init ones) or
when the driver is selected as built-in (__exit ones).

The savings are negligible, but the code readability is worse by the
order of magnitude. So put the strings where they belong. Note that it
also used to make little sense putting const data in .data (the __exit
case).

While at it, switch to pr_info, pr_err, not using the KERN_INFO and _ERR
directly.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/n_hdlc.c | 28 ++++++----------------------
 1 file changed, 6 insertions(+), 22 deletions(-)

diff --git a/drivers/tty/n_hdlc.c b/drivers/tty/n_hdlc.c
index e37dab2528a1..b651b5ba64ee 100644
--- a/drivers/tty/n_hdlc.c
+++ b/drivers/tty/n_hdlc.c
@@ -889,13 +889,6 @@ static struct n_hdlc_buf *n_hdlc_buf_get(struct n_hdlc_buf_list *buf_list)
 	return buf;
 }	/* end of n_hdlc_buf_get() */
 
-static const char hdlc_banner[] __initconst =
-	KERN_INFO "HDLC line discipline maxframe=%u\n";
-static const char hdlc_register_ok[] __initconst =
-	KERN_INFO "N_HDLC line discipline registered.\n";
-static const char hdlc_register_fail[] __initconst =
-	KERN_ERR "error registering line discipline: %d\n";
-
 static int __init n_hdlc_init(void)
 {
 	int status;
@@ -906,37 +899,28 @@ static int __init n_hdlc_init(void)
 	else if (maxframe > 65535)
 		maxframe = 65535;
 
-	printk(hdlc_banner, maxframe);
+	pr_info("HDLC line discipline maxframe=%d\n", maxframe);
 
 	status = tty_register_ldisc(N_HDLC, &n_hdlc_ldisc);
 	if (!status)
-		printk(hdlc_register_ok);
+		pr_info("N_HDLC line discipline registered.\n");
 	else
-		printk(hdlc_register_fail, status);
+		pr_err("error registering line discipline: %d\n", status);
 
 	return status;
 	
 }	/* end of init_module() */
 
-#ifdef CONFIG_SPARC
-#undef __exitdata
-#define __exitdata
-#endif
-
-static const char hdlc_unregister_ok[] __exitdata =
-	KERN_INFO "N_HDLC: line discipline unregistered\n";
-static const char hdlc_unregister_fail[] __exitdata =
-	KERN_ERR "N_HDLC: can't unregister line discipline (err = %d)\n";
-
 static void __exit n_hdlc_exit(void)
 {
 	/* Release tty registration of line discipline */
 	int status = tty_unregister_ldisc(N_HDLC);
 
 	if (status)
-		printk(hdlc_unregister_fail, status);
+		pr_err("N_HDLC: can't unregister line discipline (err = %d)\n",
+				status);
 	else
-		printk(hdlc_unregister_ok);
+		pr_info("N_HDLC: line discipline unregistered\n");
 }
 
 module_init(n_hdlc_init);
-- 
2.25.0

[PATCH 05/24] n_hdlc: cleanup messages during registration

[Jiri Slaby]

1) n_hdlc prints two lines during registration. Squeeze it into one.
2) prefix the error message with "N_HDLC: ", so that it's clear which
   ldisc failed to register.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/n_hdlc.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/n_hdlc.c b/drivers/tty/n_hdlc.c
index b651b5ba64ee..87f22e57e4f6 100644
--- a/drivers/tty/n_hdlc.c
+++ b/drivers/tty/n_hdlc.c
@@ -899,13 +899,13 @@ static int __init n_hdlc_init(void)
 	else if (maxframe > 65535)
 		maxframe = 65535;
 
-	pr_info("HDLC line discipline maxframe=%d\n", maxframe);
-
 	status = tty_register_ldisc(N_HDLC, &n_hdlc_ldisc);
 	if (!status)
-		pr_info("N_HDLC line discipline registered.\n");
+		pr_info("N_HDLC line discipline registered with maxframe=%d\n",
+				maxframe);
 	else
-		pr_err("error registering line discipline: %d\n", status);
+		pr_err("N_HDLC: error registering line discipline: %d\n",
+				status);
 
 	return status;
 	
-- 
2.25.0

[PATCH 06/24] n_hdlc: use clamp() for maxframe

[Jiri Slaby]

It is easier to read. And use MAX_HDLC_FRAME_SIZE instead of magic
constant.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/n_hdlc.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/tty/n_hdlc.c b/drivers/tty/n_hdlc.c
index 87f22e57e4f6..a713a98fea2e 100644
--- a/drivers/tty/n_hdlc.c
+++ b/drivers/tty/n_hdlc.c
@@ -894,10 +894,7 @@ static int __init n_hdlc_init(void)
 	int status;
 
 	/* range check maxframe arg */
-	if (maxframe < 4096)
-		maxframe = 4096;
-	else if (maxframe > 65535)
-		maxframe = 65535;
+	maxframe = clamp(maxframe, 4096, MAX_HDLC_FRAME_SIZE);
 
 	status = tty_register_ldisc(N_HDLC, &n_hdlc_ldisc);
 	if (!status)
-- 
2.25.0

[PATCH 07/24] n_hdlc: simplify freeing of buffer list

[Jiri Slaby]

n_hdlc_release contains four loops to free each buffer list. Create a
helper (n_hdlc_free_buf_list) and call it for every list instead. It
makes n_hdlc_release more readable.

We are switching from "for (;;)" to "do {} while (buf)" which avoids the
"if (buf)" completely -- kfree is a nop for NULL pointers.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/n_hdlc.c | 45 ++++++++++++++------------------------------
 1 file changed, 14 insertions(+), 31 deletions(-)

diff --git a/drivers/tty/n_hdlc.c b/drivers/tty/n_hdlc.c
index a713a98fea2e..07ba42badd7a 100644
--- a/drivers/tty/n_hdlc.c
+++ b/drivers/tty/n_hdlc.c
@@ -215,6 +215,16 @@ static struct tty_ldisc_ops n_hdlc_ldisc = {
 	.flush_buffer   = flush_rx_queue,
 };
 
+static void n_hdlc_free_buf_list(struct n_hdlc_buf_list *list)
+{
+	struct n_hdlc_buf *buf;
+
+	do {
+		buf = n_hdlc_buf_get(list);
+		kfree(buf);
+	} while (buf);
+}
+
 /**
  * n_hdlc_release - release an n_hdlc per device line discipline info structure
  * @n_hdlc - per device line discipline info structure
@@ -222,7 +232,6 @@ static struct tty_ldisc_ops n_hdlc_ldisc = {
 static void n_hdlc_release(struct n_hdlc *n_hdlc)
 {
 	struct tty_struct *tty = n_hdlc2tty (n_hdlc);
-	struct n_hdlc_buf *buf;
 
 	/* Ensure that the n_hdlcd process is not hanging on select()/poll() */
 	wake_up_interruptible (&tty->read_wait);
@@ -231,37 +240,11 @@ static void n_hdlc_release(struct n_hdlc *n_hdlc)
 	if (tty->disc_data == n_hdlc)
 		tty->disc_data = NULL;	/* Break the tty->n_hdlc link */
 
-	/* Release transmit and receive buffers */
-	for(;;) {
-		buf = n_hdlc_buf_get(&n_hdlc->rx_free_buf_list);
-		if (buf) {
-			kfree(buf);
-		} else
-			break;
-	}
-	for(;;) {
-		buf = n_hdlc_buf_get(&n_hdlc->tx_free_buf_list);
-		if (buf) {
-			kfree(buf);
-		} else
-			break;
-	}
-	for(;;) {
-		buf = n_hdlc_buf_get(&n_hdlc->rx_buf_list);
-		if (buf) {
-			kfree(buf);
-		} else
-			break;
-	}
-	for(;;) {
-		buf = n_hdlc_buf_get(&n_hdlc->tx_buf_list);
-		if (buf) {
-			kfree(buf);
-		} else
-			break;
-	}
+	n_hdlc_free_buf_list(&n_hdlc->rx_free_buf_list);
+	n_hdlc_free_buf_list(&n_hdlc->tx_free_buf_list);
+	n_hdlc_free_buf_list(&n_hdlc->rx_buf_list);
+	n_hdlc_free_buf_list(&n_hdlc->tx_buf_list);
 	kfree(n_hdlc);
-	
 }	/* end of n_hdlc_release() */
 
 /**
-- 
2.25.0

[PATCH 08/24] n_hdlc: invert conditions in n_hdlc_tty_close and n_hdlc_tty_poll

[Jiri Slaby]

This makes the functions return immediatelly on invalid state. And we
can push the indent of the later code one level left.

Pass "-w" to "git show" to see we are changing only the conditions (and
whitespace).

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/n_hdlc.c | 72 +++++++++++++++++++++++---------------------
 1 file changed, 38 insertions(+), 34 deletions(-)

diff --git a/drivers/tty/n_hdlc.c b/drivers/tty/n_hdlc.c
index 07ba42badd7a..e40561caa450 100644
--- a/drivers/tty/n_hdlc.c
+++ b/drivers/tty/n_hdlc.c
@@ -258,24 +258,25 @@ static void n_hdlc_tty_close(struct tty_struct *tty)
 {
 	struct n_hdlc *n_hdlc = tty2n_hdlc (tty);
 
-	if (n_hdlc != NULL) {
-		if (n_hdlc->magic != HDLC_MAGIC) {
-			printk (KERN_WARNING"n_hdlc: trying to close unopened tty!\n");
-			return;
-		}
+	if (!n_hdlc)
+		return;
+
+	if (n_hdlc->magic != HDLC_MAGIC) {
+		printk(KERN_WARNING "n_hdlc: trying to close unopened tty!\n");
+		return;
+	}
 #if defined(TTY_NO_WRITE_SPLIT)
-		clear_bit(TTY_NO_WRITE_SPLIT,&tty->flags);
+	clear_bit(TTY_NO_WRITE_SPLIT,&tty->flags);
 #endif
-		tty->disc_data = NULL;
-		if (tty == n_hdlc->backup_tty)
-			n_hdlc->backup_tty = NULL;
-		if (tty != n_hdlc->tty)
-			return;
-		if (n_hdlc->backup_tty) {
-			n_hdlc->tty = n_hdlc->backup_tty;
-		} else {
-			n_hdlc_release (n_hdlc);
-		}
+	tty->disc_data = NULL;
+	if (tty == n_hdlc->backup_tty)
+		n_hdlc->backup_tty = NULL;
+	if (tty != n_hdlc->tty)
+		return;
+	if (n_hdlc->backup_tty) {
+		n_hdlc->tty = n_hdlc->backup_tty;
+	} else {
+		n_hdlc_release (n_hdlc);
 	}
 }	/* end of n_hdlc_tty_close() */
 
@@ -737,24 +738,27 @@ static __poll_t n_hdlc_tty_poll(struct tty_struct *tty, struct file *filp,
 	struct n_hdlc *n_hdlc = tty2n_hdlc (tty);
 	__poll_t mask = 0;
 
-	if (n_hdlc && n_hdlc->magic == HDLC_MAGIC && tty == n_hdlc->tty) {
-		/* queue current process into any wait queue that */
-		/* may awaken in the future (read and write) */
-
-		poll_wait(filp, &tty->read_wait, wait);
-		poll_wait(filp, &tty->write_wait, wait);
-
-		/* set bits for operations that won't block */
-		if (!list_empty(&n_hdlc->rx_buf_list.list))
-			mask |= EPOLLIN | EPOLLRDNORM;	/* readable */
-		if (test_bit(TTY_OTHER_CLOSED, &tty->flags))
-			mask |= EPOLLHUP;
-		if (tty_hung_up_p(filp))
-			mask |= EPOLLHUP;
-		if (!tty_is_writelocked(tty) &&
-				!list_empty(&n_hdlc->tx_free_buf_list.list))
-			mask |= EPOLLOUT | EPOLLWRNORM;	/* writable */
-	}
+	if (!n_hdlc || n_hdlc->magic != HDLC_MAGIC || tty != n_hdlc->tty)
+		return 0;
+
+	/*
+	 * queue the current process into any wait queue that may awaken in the
+	 * future (read and write)
+	 */
+	poll_wait(filp, &tty->read_wait, wait);
+	poll_wait(filp, &tty->write_wait, wait);
+
+	/* set bits for operations that won't block */
+	if (!list_empty(&n_hdlc->rx_buf_list.list))
+		mask |= EPOLLIN | EPOLLRDNORM;	/* readable */
+	if (test_bit(TTY_OTHER_CLOSED, &tty->flags))
+		mask |= EPOLLHUP;
+	if (tty_hung_up_p(filp))
+		mask |= EPOLLHUP;
+	if (!tty_is_writelocked(tty) &&
+			!list_empty(&n_hdlc->tx_free_buf_list.list))
+		mask |= EPOLLOUT | EPOLLWRNORM;	/* writable */
+
 	return mask;
 }	/* end of n_hdlc_tty_poll() */
 
-- 
2.25.0

[PATCH 09/24] n_hdlc: remove unused flags

[Jiri Slaby]

They are only set to 0 and never read.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/n_hdlc.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/tty/n_hdlc.c b/drivers/tty/n_hdlc.c
index e40561caa450..d0e538ba51ea 100644
--- a/drivers/tty/n_hdlc.c
+++ b/drivers/tty/n_hdlc.c
@@ -124,7 +124,6 @@ struct n_hdlc_buf_list {
 /**
  * struct n_hdlc - per device instance data structure
  * @magic - magic value for structure
- * @flags - miscellaneous control flags
  * @tty - ptr to TTY structure
  * @backup_tty - TTY to use if tty gets closed
  * @tbusy - reentrancy flag for tx wakeup code
@@ -136,7 +135,6 @@ struct n_hdlc_buf_list {
  */
 struct n_hdlc {
 	int			magic;
-	__u32			flags;
 	struct tty_struct	*tty;
 	struct tty_struct	*backup_tty;
 	int			tbusy;
@@ -808,8 +806,7 @@ static struct n_hdlc *n_hdlc_alloc(void)
 	
 	/* Initialize the control block */
 	n_hdlc->magic  = HDLC_MAGIC;
-	n_hdlc->flags  = 0;
-	
+
 	return n_hdlc;
 	
 }	/* end of n_hdlc_alloc() */
-- 
2.25.0

[PATCH 10/24] n_hdlc: remove unused backup_tty

[Jiri Slaby]

It's only set to NULL and never properly used.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/n_hdlc.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/tty/n_hdlc.c b/drivers/tty/n_hdlc.c
index d0e538ba51ea..ae6b9a45ae81 100644
--- a/drivers/tty/n_hdlc.c
+++ b/drivers/tty/n_hdlc.c
@@ -125,7 +125,6 @@ struct n_hdlc_buf_list {
  * struct n_hdlc - per device instance data structure
  * @magic - magic value for structure
  * @tty - ptr to TTY structure
- * @backup_tty - TTY to use if tty gets closed
  * @tbusy - reentrancy flag for tx wakeup code
  * @woke_up - FIXME: describe this field
  * @tx_buf_list - list of pending transmit frame buffers
@@ -136,7 +135,6 @@ struct n_hdlc_buf_list {
 struct n_hdlc {
 	int			magic;
 	struct tty_struct	*tty;
-	struct tty_struct	*backup_tty;
 	int			tbusy;
 	int			woke_up;
 	struct n_hdlc_buf_list	tx_buf_list;
@@ -267,15 +265,9 @@ static void n_hdlc_tty_close(struct tty_struct *tty)
 	clear_bit(TTY_NO_WRITE_SPLIT,&tty->flags);
 #endif
 	tty->disc_data = NULL;
-	if (tty == n_hdlc->backup_tty)
-		n_hdlc->backup_tty = NULL;
 	if (tty != n_hdlc->tty)
 		return;
-	if (n_hdlc->backup_tty) {
-		n_hdlc->tty = n_hdlc->backup_tty;
-	} else {
-		n_hdlc_release (n_hdlc);
-	}
+	n_hdlc_release (n_hdlc);
 }	/* end of n_hdlc_tty_close() */
 
 /**
-- 
2.25.0

[PATCH 11/24] n_hdlc: expand tty2n_hdlc macro

[Jiri Slaby]

It's simple tty->disc_data, but it obfuscates code. So expand it to all
locations and drop it.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/n_hdlc.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/drivers/tty/n_hdlc.c b/drivers/tty/n_hdlc.c
index ae6b9a45ae81..67162611af58 100644
--- a/drivers/tty/n_hdlc.c
+++ b/drivers/tty/n_hdlc.c
@@ -175,12 +175,11 @@ static void n_hdlc_tty_receive(struct tty_struct *tty, const __u8 *cp,
 			       char *fp, int count);
 static void n_hdlc_tty_wakeup(struct tty_struct *tty);
 
-#define tty2n_hdlc(tty)	((struct n_hdlc *) ((tty)->disc_data))
 #define n_hdlc2tty(n_hdlc)	((n_hdlc)->tty)
 
 static void flush_rx_queue(struct tty_struct *tty)
 {
-	struct n_hdlc *n_hdlc = tty2n_hdlc(tty);
+	struct n_hdlc *n_hdlc = tty->disc_data;
 	struct n_hdlc_buf *buf;
 
 	while ((buf = n_hdlc_buf_get(&n_hdlc->rx_buf_list)))
@@ -189,7 +188,7 @@ static void flush_rx_queue(struct tty_struct *tty)
 
 static void flush_tx_queue(struct tty_struct *tty)
 {
-	struct n_hdlc *n_hdlc = tty2n_hdlc(tty);
+	struct n_hdlc *n_hdlc = tty->disc_data;
 	struct n_hdlc_buf *buf;
 
 	while ((buf = n_hdlc_buf_get(&n_hdlc->tx_buf_list)))
@@ -252,7 +251,7 @@ static void n_hdlc_release(struct n_hdlc *n_hdlc)
  */
 static void n_hdlc_tty_close(struct tty_struct *tty)
 {
-	struct n_hdlc *n_hdlc = tty2n_hdlc (tty);
+	struct n_hdlc *n_hdlc = tty->disc_data;
 
 	if (!n_hdlc)
 		return;
@@ -278,7 +277,7 @@ static void n_hdlc_tty_close(struct tty_struct *tty)
  */
 static int n_hdlc_tty_open (struct tty_struct *tty)
 {
-	struct n_hdlc *n_hdlc = tty2n_hdlc (tty);
+	struct n_hdlc *n_hdlc = tty->disc_data;
 
 	pr_debug("%s(%d)%s() called (device=%s)\n",
 			__FILE__, __LINE__, __func__, tty->name);
@@ -402,7 +401,7 @@ static void n_hdlc_send_frames(struct n_hdlc *n_hdlc, struct tty_struct *tty)
  */
 static void n_hdlc_tty_wakeup(struct tty_struct *tty)
 {
-	struct n_hdlc *n_hdlc = tty2n_hdlc(tty);
+	struct n_hdlc *n_hdlc = tty->disc_data;
 
 	if (!n_hdlc)
 		return;
@@ -429,7 +428,7 @@ static void n_hdlc_tty_wakeup(struct tty_struct *tty)
 static void n_hdlc_tty_receive(struct tty_struct *tty, const __u8 *data,
 			       char *flags, int count)
 {
-	register struct n_hdlc *n_hdlc = tty2n_hdlc (tty);
+	register struct n_hdlc *n_hdlc = tty->disc_data;
 	register struct n_hdlc_buf *buf;
 
 	pr_debug("%s(%d)%s() called count=%d\n",
@@ -494,7 +493,7 @@ static void n_hdlc_tty_receive(struct tty_struct *tty, const __u8 *data,
 static ssize_t n_hdlc_tty_read(struct tty_struct *tty, struct file *file,
 			   __u8 __user *buf, size_t nr)
 {
-	struct n_hdlc *n_hdlc = tty2n_hdlc(tty);
+	struct n_hdlc *n_hdlc = tty->disc_data;
 	int ret = 0;
 	struct n_hdlc_buf *rbuf;
 	DECLARE_WAITQUEUE(wait, current);
@@ -576,7 +575,7 @@ static ssize_t n_hdlc_tty_read(struct tty_struct *tty, struct file *file,
 static ssize_t n_hdlc_tty_write(struct tty_struct *tty, struct file *file,
 			    const unsigned char *data, size_t count)
 {
-	struct n_hdlc *n_hdlc = tty2n_hdlc (tty);
+	struct n_hdlc *n_hdlc = tty->disc_data;
 	int error = 0;
 	DECLARE_WAITQUEUE(wait, current);
 	struct n_hdlc_buf *tbuf;
@@ -613,7 +612,7 @@ static ssize_t n_hdlc_tty_write(struct tty_struct *tty, struct file *file,
 		}
 		schedule();
 			
-		n_hdlc = tty2n_hdlc (tty);
+		n_hdlc = tty->disc_data;
 		if (!n_hdlc || n_hdlc->magic != HDLC_MAGIC || 
 		    tty != n_hdlc->tty) {
 			printk("n_hdlc_tty_write: %p invalid after wait!\n", n_hdlc);
@@ -656,7 +655,7 @@ static ssize_t n_hdlc_tty_write(struct tty_struct *tty, struct file *file,
 static int n_hdlc_tty_ioctl(struct tty_struct *tty, struct file *file,
 			    unsigned int cmd, unsigned long arg)
 {
-	struct n_hdlc *n_hdlc = tty2n_hdlc (tty);
+	struct n_hdlc *n_hdlc = tty->disc_data;
 	int error = 0;
 	int count;
 	unsigned long flags;
@@ -725,7 +724,7 @@ static int n_hdlc_tty_ioctl(struct tty_struct *tty, struct file *file,
 static __poll_t n_hdlc_tty_poll(struct tty_struct *tty, struct file *filp,
 				    poll_table *wait)
 {
-	struct n_hdlc *n_hdlc = tty2n_hdlc (tty);
+	struct n_hdlc *n_hdlc = tty->disc_data;
 	__poll_t mask = 0;
 
 	if (!n_hdlc || n_hdlc->magic != HDLC_MAGIC || tty != n_hdlc->tty)
-- 
2.25.0

[PATCH 12/24] n_hdlc: inline n_hdlc_release

[Jiri Slaby]

Put the body of n_hdlc_release into the only caller. It can be seen,
that the "if" is superfluous now -- the same happens few lines above in
n_hdlc_tty_close already. So drop it.

Drop also n_hdlc2tty macro as this was the only user.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/n_hdlc.c | 35 ++++++++++-------------------------
 1 file changed, 10 insertions(+), 25 deletions(-)

diff --git a/drivers/tty/n_hdlc.c b/drivers/tty/n_hdlc.c
index 67162611af58..39a58febd896 100644
--- a/drivers/tty/n_hdlc.c
+++ b/drivers/tty/n_hdlc.c
@@ -175,8 +175,6 @@ static void n_hdlc_tty_receive(struct tty_struct *tty, const __u8 *cp,
 			       char *fp, int count);
 static void n_hdlc_tty_wakeup(struct tty_struct *tty);
 
-#define n_hdlc2tty(n_hdlc)	((n_hdlc)->tty)
-
 static void flush_rx_queue(struct tty_struct *tty)
 {
 	struct n_hdlc *n_hdlc = tty->disc_data;
@@ -220,28 +218,6 @@ static void n_hdlc_free_buf_list(struct n_hdlc_buf_list *list)
 	} while (buf);
 }
 
-/**
- * n_hdlc_release - release an n_hdlc per device line discipline info structure
- * @n_hdlc - per device line discipline info structure
- */
-static void n_hdlc_release(struct n_hdlc *n_hdlc)
-{
-	struct tty_struct *tty = n_hdlc2tty (n_hdlc);
-
-	/* Ensure that the n_hdlcd process is not hanging on select()/poll() */
-	wake_up_interruptible (&tty->read_wait);
-	wake_up_interruptible (&tty->write_wait);
-
-	if (tty->disc_data == n_hdlc)
-		tty->disc_data = NULL;	/* Break the tty->n_hdlc link */
-
-	n_hdlc_free_buf_list(&n_hdlc->rx_free_buf_list);
-	n_hdlc_free_buf_list(&n_hdlc->tx_free_buf_list);
-	n_hdlc_free_buf_list(&n_hdlc->rx_buf_list);
-	n_hdlc_free_buf_list(&n_hdlc->tx_buf_list);
-	kfree(n_hdlc);
-}	/* end of n_hdlc_release() */
-
 /**
  * n_hdlc_tty_close - line discipline close
  * @tty - pointer to tty info structure
@@ -266,7 +242,16 @@ static void n_hdlc_tty_close(struct tty_struct *tty)
 	tty->disc_data = NULL;
 	if (tty != n_hdlc->tty)
 		return;
-	n_hdlc_release (n_hdlc);
+
+	/* Ensure that the n_hdlcd process is not hanging on select()/poll() */
+	wake_up_interruptible(&tty->read_wait);
+	wake_up_interruptible(&tty->write_wait);
+
+	n_hdlc_free_buf_list(&n_hdlc->rx_free_buf_list);
+	n_hdlc_free_buf_list(&n_hdlc->tx_free_buf_list);
+	n_hdlc_free_buf_list(&n_hdlc->rx_buf_list);
+	n_hdlc_free_buf_list(&n_hdlc->tx_buf_list);
+	kfree(n_hdlc);
 }	/* end of n_hdlc_tty_close() */
 
 /**
-- 
2.25.0

[PATCH 13/24] n_hdlc: remove cached tty

[Jiri Slaby]

It's not needed, as now it's clear, that it's always the same as the one
passed from the tty layer.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/n_hdlc.c | 21 +++++----------------
 1 file changed, 5 insertions(+), 16 deletions(-)

diff --git a/drivers/tty/n_hdlc.c b/drivers/tty/n_hdlc.c
index 39a58febd896..2ac702974b57 100644
--- a/drivers/tty/n_hdlc.c
+++ b/drivers/tty/n_hdlc.c
@@ -124,7 +124,6 @@ struct n_hdlc_buf_list {
 /**
  * struct n_hdlc - per device instance data structure
  * @magic - magic value for structure
- * @tty - ptr to TTY structure
  * @tbusy - reentrancy flag for tx wakeup code
  * @woke_up - FIXME: describe this field
  * @tx_buf_list - list of pending transmit frame buffers
@@ -134,7 +133,6 @@ struct n_hdlc_buf_list {
  */
 struct n_hdlc {
 	int			magic;
-	struct tty_struct	*tty;
 	int			tbusy;
 	int			woke_up;
 	struct n_hdlc_buf_list	tx_buf_list;
@@ -240,8 +238,6 @@ static void n_hdlc_tty_close(struct tty_struct *tty)
 	clear_bit(TTY_NO_WRITE_SPLIT,&tty->flags);
 #endif
 	tty->disc_data = NULL;
-	if (tty != n_hdlc->tty)
-		return;
 
 	/* Ensure that the n_hdlcd process is not hanging on select()/poll() */
 	wake_up_interruptible(&tty->read_wait);
@@ -280,7 +276,6 @@ static int n_hdlc_tty_open (struct tty_struct *tty)
 	}
 		
 	tty->disc_data = n_hdlc;
-	n_hdlc->tty    = tty;
 	tty->receive_room = 65536;
 	
 #if defined(TTY_NO_WRITE_SPLIT)
@@ -391,11 +386,6 @@ static void n_hdlc_tty_wakeup(struct tty_struct *tty)
 	if (!n_hdlc)
 		return;
 
-	if (tty != n_hdlc->tty) {
-		clear_bit(TTY_DO_WRITE_WAKEUP, &tty->flags);
-		return;
-	}
-
 	n_hdlc_send_frames (n_hdlc, tty);
 		
 }	/* end of n_hdlc_tty_wakeup() */
@@ -420,7 +410,7 @@ static void n_hdlc_tty_receive(struct tty_struct *tty, const __u8 *data,
 			__FILE__, __LINE__, __func__, count);
 
 	/* This can happen if stuff comes in on the backup tty */
-	if (!n_hdlc || tty != n_hdlc->tty)
+	if (!n_hdlc)
 		return;
 		
 	/* verify line is using HDLC discipline */
@@ -461,8 +451,8 @@ static void n_hdlc_tty_receive(struct tty_struct *tty, const __u8 *data,
 	
 	/* wake up any blocked reads and perform async signalling */
 	wake_up_interruptible (&tty->read_wait);
-	if (n_hdlc->tty->fasync != NULL)
-		kill_fasync (&n_hdlc->tty->fasync, SIGIO, POLL_IN);
+	if (tty->fasync != NULL)
+		kill_fasync(&tty->fasync, SIGIO, POLL_IN);
 
 }	/* end of n_hdlc_tty_receive() */
 
@@ -598,8 +588,7 @@ static ssize_t n_hdlc_tty_write(struct tty_struct *tty, struct file *file,
 		schedule();
 			
 		n_hdlc = tty->disc_data;
-		if (!n_hdlc || n_hdlc->magic != HDLC_MAGIC || 
-		    tty != n_hdlc->tty) {
+		if (!n_hdlc || n_hdlc->magic != HDLC_MAGIC) {
 			printk("n_hdlc_tty_write: %p invalid after wait!\n", n_hdlc);
 			error = -EIO;
 			break;
@@ -712,7 +701,7 @@ static __poll_t n_hdlc_tty_poll(struct tty_struct *tty, struct file *filp,
 	struct n_hdlc *n_hdlc = tty->disc_data;
 	__poll_t mask = 0;
 
-	if (!n_hdlc || n_hdlc->magic != HDLC_MAGIC || tty != n_hdlc->tty)
+	if (!n_hdlc || n_hdlc->magic != HDLC_MAGIC)
 		return 0;
 
 	/*
-- 
2.25.0

[PATCH 14/24] n_hdlc: remove checking of n_hdlc

[Jiri Slaby]

We got rid of backup_tty recently. Also, the tty layer ensures not to
call other ldisc hooks after ldisc close. That means, all those tests
are superfluous now so remove them.

Note that we remove the magic check in write after schedule too. The tty
cannot change during schedule.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/n_hdlc.c | 32 +++-----------------------------
 1 file changed, 3 insertions(+), 29 deletions(-)

diff --git a/drivers/tty/n_hdlc.c b/drivers/tty/n_hdlc.c
index 2ac702974b57..2709d18364eb 100644
--- a/drivers/tty/n_hdlc.c
+++ b/drivers/tty/n_hdlc.c
@@ -227,9 +227,6 @@ static void n_hdlc_tty_close(struct tty_struct *tty)
 {
 	struct n_hdlc *n_hdlc = tty->disc_data;
 
-	if (!n_hdlc)
-		return;
-
 	if (n_hdlc->magic != HDLC_MAGIC) {
 		printk(KERN_WARNING "n_hdlc: trying to close unopened tty!\n");
 		return;
@@ -383,11 +380,7 @@ static void n_hdlc_tty_wakeup(struct tty_struct *tty)
 {
 	struct n_hdlc *n_hdlc = tty->disc_data;
 
-	if (!n_hdlc)
-		return;
-
 	n_hdlc_send_frames (n_hdlc, tty);
-		
 }	/* end of n_hdlc_tty_wakeup() */
 
 /**
@@ -409,10 +402,6 @@ static void n_hdlc_tty_receive(struct tty_struct *tty, const __u8 *data,
 	pr_debug("%s(%d)%s() called count=%d\n",
 			__FILE__, __LINE__, __func__, count);
 
-	/* This can happen if stuff comes in on the backup tty */
-	if (!n_hdlc)
-		return;
-		
 	/* verify line is using HDLC discipline */
 	if (n_hdlc->magic != HDLC_MAGIC) {
 		printk("%s(%d) line not using HDLC discipline\n",
@@ -473,10 +462,6 @@ static ssize_t n_hdlc_tty_read(struct tty_struct *tty, struct file *file,
 	struct n_hdlc_buf *rbuf;
 	DECLARE_WAITQUEUE(wait, current);
 
-	/* Validate the pointers */
-	if (!n_hdlc)
-		return -EIO;
-
 	/* verify user access to buffer */
 	if (!access_ok(buf, nr)) {
 		printk(KERN_WARNING "%s(%d) n_hdlc_tty_read() can't verify user "
@@ -558,10 +543,6 @@ static ssize_t n_hdlc_tty_write(struct tty_struct *tty, struct file *file,
 	pr_debug("%s(%d)%s() called count=%zd\n", __FILE__, __LINE__, __func__,
 			count);
 
-	/* Verify pointers */
-	if (!n_hdlc)
-		return -EIO;
-
 	if (n_hdlc->magic != HDLC_MAGIC)
 		return -EIO;
 
@@ -586,14 +567,7 @@ static ssize_t n_hdlc_tty_write(struct tty_struct *tty, struct file *file,
 			break;
 		}
 		schedule();
-			
-		n_hdlc = tty->disc_data;
-		if (!n_hdlc || n_hdlc->magic != HDLC_MAGIC) {
-			printk("n_hdlc_tty_write: %p invalid after wait!\n", n_hdlc);
-			error = -EIO;
-			break;
-		}
-			
+
 		if (signal_pending(current)) {
 			error = -EINTR;
 			break;
@@ -638,7 +612,7 @@ static int n_hdlc_tty_ioctl(struct tty_struct *tty, struct file *file,
 	pr_debug("%s(%d)%s() called %d\n", __FILE__, __LINE__, __func__, cmd);
 
 	/* Verify the status of the device */
-	if (!n_hdlc || n_hdlc->magic != HDLC_MAGIC)
+	if (n_hdlc->magic != HDLC_MAGIC)
 		return -EBADF;
 
 	switch (cmd) {
@@ -701,7 +675,7 @@ static __poll_t n_hdlc_tty_poll(struct tty_struct *tty, struct file *filp,
 	struct n_hdlc *n_hdlc = tty->disc_data;
 	__poll_t mask = 0;
 
-	if (!n_hdlc || n_hdlc->magic != HDLC_MAGIC)
+	if (n_hdlc->magic != HDLC_MAGIC)
 		return 0;
 
 	/*
-- 
2.25.0

[PATCH 15/24] n_hdlc: add helper for buffers allocation

[Jiri Slaby]

Given both rx and tx allocations do the same, add a new helper
(n_hdlc_alloc_buf) and use it for both of them. This cleans up
n_hdlc_alloc slightly.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/n_hdlc.c | 42 ++++++++++++++++++++----------------------
 1 file changed, 20 insertions(+), 22 deletions(-)

diff --git a/drivers/tty/n_hdlc.c b/drivers/tty/n_hdlc.c
index 2709d18364eb..4276931fd071 100644
--- a/drivers/tty/n_hdlc.c
+++ b/drivers/tty/n_hdlc.c
@@ -699,6 +699,23 @@ static __poll_t n_hdlc_tty_poll(struct tty_struct *tty, struct file *filp,
 	return mask;
 }	/* end of n_hdlc_tty_poll() */
 
+static void n_hdlc_alloc_buf(struct n_hdlc_buf_list *list, unsigned int count,
+		const char *name)
+{
+	struct n_hdlc_buf *buf;
+	unsigned int i;
+
+	for (i = 0; i < count; i++) {
+		buf = kmalloc(struct_size(buf, buf, maxframe), GFP_KERNEL);
+		if (!buf) {
+			pr_debug("%s(%d)%s(), kmalloc() failed for %s buffer %u\n",
+					__FILE__, __LINE__, __func__, name, i);
+			return;
+		}
+		n_hdlc_buf_put(list, buf);
+	}
+}
+
 /**
  * n_hdlc_alloc - allocate an n_hdlc instance data structure
  *
@@ -706,8 +723,6 @@ static __poll_t n_hdlc_tty_poll(struct tty_struct *tty, struct file *filp,
  */
 static struct n_hdlc *n_hdlc_alloc(void)
 {
-	struct n_hdlc_buf *buf;
-	int i;
 	struct n_hdlc *n_hdlc = kzalloc(sizeof(*n_hdlc), GFP_KERNEL);
 
 	if (!n_hdlc)
@@ -723,26 +738,9 @@ static struct n_hdlc *n_hdlc_alloc(void)
 	INIT_LIST_HEAD(&n_hdlc->rx_buf_list.list);
 	INIT_LIST_HEAD(&n_hdlc->tx_buf_list.list);
 
-	/* allocate free rx buffer list */
-	for(i=0;i<DEFAULT_RX_BUF_COUNT;i++) {
-		buf = kmalloc(struct_size(buf, buf, maxframe), GFP_KERNEL);
-		if (buf)
-			n_hdlc_buf_put(&n_hdlc->rx_free_buf_list,buf);
-		else
-			pr_debug("%s(%d)%s(), kmalloc() failed for rx buffer %d\n",
-					__FILE__, __LINE__, __func__, i);
-	}
-	
-	/* allocate free tx buffer list */
-	for(i=0;i<DEFAULT_TX_BUF_COUNT;i++) {
-		buf = kmalloc(struct_size(buf, buf, maxframe), GFP_KERNEL);
-		if (buf)
-			n_hdlc_buf_put(&n_hdlc->tx_free_buf_list,buf);
-		else
-			pr_debug("%s(%d)%s(), kmalloc() failed for tx buffer %d\n",
-					__FILE__, __LINE__, __func__, i);
-	}
-	
+	n_hdlc_alloc_buf(&n_hdlc->rx_free_buf_list, DEFAULT_RX_BUF_COUNT, "rx");
+	n_hdlc_alloc_buf(&n_hdlc->tx_free_buf_list, DEFAULT_TX_BUF_COUNT, "tx");
+
 	/* Initialize the control block */
 	n_hdlc->magic  = HDLC_MAGIC;
 
-- 
2.25.0

[PATCH 16/24] n_hdlc: move tty_ldisc_ops to the bottom

[Jiri Slaby]

This makes forward declarations unnecessary. So drop them.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/n_hdlc.c | 46 +++++++++++++++-----------------------------
 1 file changed, 15 insertions(+), 31 deletions(-)

diff --git a/drivers/tty/n_hdlc.c b/drivers/tty/n_hdlc.c
index 4276931fd071..615abe582cac 100644
--- a/drivers/tty/n_hdlc.c
+++ b/drivers/tty/n_hdlc.c
@@ -157,22 +157,6 @@ static struct n_hdlc *n_hdlc_alloc (void);
 /* max frame size for memory allocations */
 static int maxframe = 4096;
 
-/* TTY callbacks */
-
-static ssize_t n_hdlc_tty_read(struct tty_struct *tty, struct file *file,
-			   __u8 __user *buf, size_t nr);
-static ssize_t n_hdlc_tty_write(struct tty_struct *tty, struct file *file,
-			    const unsigned char *buf, size_t nr);
-static int n_hdlc_tty_ioctl(struct tty_struct *tty, struct file *file,
-			    unsigned int cmd, unsigned long arg);
-static __poll_t n_hdlc_tty_poll(struct tty_struct *tty, struct file *filp,
-				    poll_table *wait);
-static int n_hdlc_tty_open(struct tty_struct *tty);
-static void n_hdlc_tty_close(struct tty_struct *tty);
-static void n_hdlc_tty_receive(struct tty_struct *tty, const __u8 *cp,
-			       char *fp, int count);
-static void n_hdlc_tty_wakeup(struct tty_struct *tty);
-
 static void flush_rx_queue(struct tty_struct *tty)
 {
 	struct n_hdlc *n_hdlc = tty->disc_data;
@@ -191,21 +175,6 @@ static void flush_tx_queue(struct tty_struct *tty)
 		n_hdlc_buf_put(&n_hdlc->tx_free_buf_list, buf);
 }
 
-static struct tty_ldisc_ops n_hdlc_ldisc = {
-	.owner		= THIS_MODULE,
-	.magic		= TTY_LDISC_MAGIC,
-	.name		= "hdlc",
-	.open		= n_hdlc_tty_open,
-	.close		= n_hdlc_tty_close,
-	.read		= n_hdlc_tty_read,
-	.write		= n_hdlc_tty_write,
-	.ioctl		= n_hdlc_tty_ioctl,
-	.poll		= n_hdlc_tty_poll,
-	.receive_buf	= n_hdlc_tty_receive,
-	.write_wakeup	= n_hdlc_tty_wakeup,
-	.flush_buffer   = flush_rx_queue,
-};
-
 static void n_hdlc_free_buf_list(struct n_hdlc_buf_list *list)
 {
 	struct n_hdlc_buf *buf;
@@ -810,6 +779,21 @@ static struct n_hdlc_buf *n_hdlc_buf_get(struct n_hdlc_buf_list *buf_list)
 	return buf;
 }	/* end of n_hdlc_buf_get() */
 
+static struct tty_ldisc_ops n_hdlc_ldisc = {
+	.owner		= THIS_MODULE,
+	.magic		= TTY_LDISC_MAGIC,
+	.name		= "hdlc",
+	.open		= n_hdlc_tty_open,
+	.close		= n_hdlc_tty_close,
+	.read		= n_hdlc_tty_read,
+	.write		= n_hdlc_tty_write,
+	.ioctl		= n_hdlc_tty_ioctl,
+	.poll		= n_hdlc_tty_poll,
+	.receive_buf	= n_hdlc_tty_receive,
+	.write_wakeup	= n_hdlc_tty_wakeup,
+	.flush_buffer   = flush_rx_queue,
+};
+
 static int __init n_hdlc_init(void)
 {
 	int status;
-- 
2.25.0

[PATCH 17/24] n_hdlc: switch tbusy and woke_up to bools

[Jiri Slaby]

They are in fact bools, so save some bytes (8B on x86_64). Also describe
@woke_up as we know what it is.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/n_hdlc.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/tty/n_hdlc.c b/drivers/tty/n_hdlc.c
index 615abe582cac..c4fe09124185 100644
--- a/drivers/tty/n_hdlc.c
+++ b/drivers/tty/n_hdlc.c
@@ -125,7 +125,7 @@ struct n_hdlc_buf_list {
  * struct n_hdlc - per device instance data structure
  * @magic - magic value for structure
  * @tbusy - reentrancy flag for tx wakeup code
- * @woke_up - FIXME: describe this field
+ * @woke_up - tx wakeup needs to be run again as it was called while @tbusy
  * @tx_buf_list - list of pending transmit frame buffers
  * @rx_buf_list - list of received frame buffers
  * @tx_free_buf_list - list unused transmit frame buffers
@@ -133,8 +133,8 @@ struct n_hdlc_buf_list {
  */
 struct n_hdlc {
 	int			magic;
-	int			tbusy;
-	int			woke_up;
+	bool			tbusy;
+	bool			woke_up;
 	struct n_hdlc_buf_list	tx_buf_list;
 	struct n_hdlc_buf_list	rx_buf_list;
 	struct n_hdlc_buf_list	tx_free_buf_list;
@@ -275,12 +275,12 @@ static void n_hdlc_send_frames(struct n_hdlc *n_hdlc, struct tty_struct *tty)
 		
  	spin_lock_irqsave(&n_hdlc->tx_buf_list.spinlock, flags);
 	if (n_hdlc->tbusy) {
-		n_hdlc->woke_up = 1;
- 		spin_unlock_irqrestore(&n_hdlc->tx_buf_list.spinlock, flags);
+		n_hdlc->woke_up = true;
+		spin_unlock_irqrestore(&n_hdlc->tx_buf_list.spinlock, flags);
 		return;
 	}
-	n_hdlc->tbusy = 1;
-	n_hdlc->woke_up = 0;
+	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);
@@ -332,7 +332,7 @@ static void n_hdlc_send_frames(struct n_hdlc *n_hdlc, struct tty_struct *tty)
 	
 	/* Clear the re-entry flag */
 	spin_lock_irqsave(&n_hdlc->tx_buf_list.spinlock, flags);
-	n_hdlc->tbusy = 0;
+	n_hdlc->tbusy = false;
 	spin_unlock_irqrestore(&n_hdlc->tx_buf_list.spinlock, flags); 
 	
         if (n_hdlc->woke_up)
-- 
2.25.0

[PATCH 18/24] n_hdlc: remove unneeded ifdef

[Jiri Slaby]

TTY_NO_WRITE_SPLIT is (always) defined in linux/tty.h, so no need to
check for it.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/n_hdlc.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/tty/n_hdlc.c b/drivers/tty/n_hdlc.c
index c4fe09124185..cf8485beaccd 100644
--- a/drivers/tty/n_hdlc.c
+++ b/drivers/tty/n_hdlc.c
@@ -244,10 +244,8 @@ static int n_hdlc_tty_open (struct tty_struct *tty)
 	tty->disc_data = n_hdlc;
 	tty->receive_room = 65536;
 	
-#if defined(TTY_NO_WRITE_SPLIT)
 	/* change tty_io write() to not split large writes into 8K chunks */
 	set_bit(TTY_NO_WRITE_SPLIT,&tty->flags);
-#endif
 	
 	/* flush receive data from driver */
 	tty_driver_flush_buffer(tty);
-- 
2.25.0

[PATCH 19/24] n_hdlc: use __func__ and pr_ print helpers

[Jiri Slaby]

* we mark the message in n_hdlc_tty_receive as error
* we use __func__ instead of explicit function name
* we switch the remaining prints to pr_* helpers

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/n_hdlc.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/tty/n_hdlc.c b/drivers/tty/n_hdlc.c
index cf8485beaccd..048e5a155bea 100644
--- a/drivers/tty/n_hdlc.c
+++ b/drivers/tty/n_hdlc.c
@@ -197,7 +197,7 @@ static void n_hdlc_tty_close(struct tty_struct *tty)
 	struct n_hdlc *n_hdlc = tty->disc_data;
 
 	if (n_hdlc->magic != HDLC_MAGIC) {
-		printk(KERN_WARNING "n_hdlc: trying to close unopened tty!\n");
+		pr_warn("n_hdlc: trying to close unopened tty!\n");
 		return;
 	}
 #if defined(TTY_NO_WRITE_SPLIT)
@@ -231,13 +231,13 @@ static int n_hdlc_tty_open (struct tty_struct *tty)
 
 	/* There should not be an existing table for this slot. */
 	if (n_hdlc) {
-		printk (KERN_ERR"n_hdlc_tty_open:tty already associated!\n" );
+		pr_err("%s: tty already associated!\n", __func__);
 		return -EEXIST;
 	}
 	
 	n_hdlc = n_hdlc_alloc();
 	if (!n_hdlc) {
-		printk (KERN_ERR "n_hdlc_alloc failed\n");
+		pr_err("%s: n_hdlc_alloc failed\n", __func__);
 		return -ENFILE;
 	}
 		
@@ -371,8 +371,8 @@ static void n_hdlc_tty_receive(struct tty_struct *tty, const __u8 *data,
 
 	/* verify line is using HDLC discipline */
 	if (n_hdlc->magic != HDLC_MAGIC) {
-		printk("%s(%d) line not using HDLC discipline\n",
-			__FILE__,__LINE__);
+		pr_err("%s(%d) line not using HDLC discipline\n",
+				__FILE__, __LINE__);
 		return;
 	}
 	
@@ -431,8 +431,8 @@ static ssize_t n_hdlc_tty_read(struct tty_struct *tty, struct file *file,
 
 	/* verify user access to buffer */
 	if (!access_ok(buf, nr)) {
-		printk(KERN_WARNING "%s(%d) n_hdlc_tty_read() can't verify user "
-		"buffer\n", __FILE__, __LINE__);
+		pr_warn("%s(%d) %s() can't verify user buffer\n",
+				__FILE__, __LINE__, __func__);
 		return -EFAULT;
 	}
 
-- 
2.25.0

[PATCH 20/24] n_hdlc: remove useless whitespace at line wraps

[Jiri Slaby]

Do
  s@[ \t]\+$@@
  s@ \+\t@\t@
on the file as there are many spaces at the begininning of lines and
many spaces/tabs at EOLs. And vim screamed.

git show -w is supposed to show no difference here.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/n_hdlc.c | 74 ++++++++++++++++++++++----------------------
 1 file changed, 37 insertions(+), 37 deletions(-)

diff --git a/drivers/tty/n_hdlc.c b/drivers/tty/n_hdlc.c
index 048e5a155bea..ee27573b0624 100644
--- a/drivers/tty/n_hdlc.c
+++ b/drivers/tty/n_hdlc.c
@@ -18,7 +18,7 @@
  * All HDLC data is frame oriented which means:
  *
  * 1. tty write calls represent one complete transmit frame of data
- *    The device driver should accept the complete frame or none of 
+ *    The device driver should accept the complete frame or none of
  *    the frame (busy) in the write method. Each write call should have
  *    a byte count in the range of 2-65535 bytes (2 is min HDLC frame
  *    with 1 addr byte and 1 ctrl byte). The max byte count of 65535
@@ -39,7 +39,7 @@
  *    tty read calls.
  *
  * 3. tty read calls returns an entire frame of data or nothing.
- *    
+ *
  * 4. all send and receive data is considered raw. No processing
  *    or translation is performed by the line discipline, regardless
  *    of the tty flags
@@ -104,7 +104,7 @@
 /*
  * Buffers for individual HDLC frames
  */
-#define MAX_HDLC_FRAME_SIZE 65535 
+#define MAX_HDLC_FRAME_SIZE 65535
 #define DEFAULT_RX_BUF_COUNT 10
 #define MAX_RX_BUF_COUNT 60
 #define DEFAULT_TX_BUF_COUNT 3
@@ -234,24 +234,24 @@ static int n_hdlc_tty_open (struct tty_struct *tty)
 		pr_err("%s: tty already associated!\n", __func__);
 		return -EEXIST;
 	}
-	
+
 	n_hdlc = n_hdlc_alloc();
 	if (!n_hdlc) {
 		pr_err("%s: n_hdlc_alloc failed\n", __func__);
 		return -ENFILE;
 	}
-		
+
 	tty->disc_data = n_hdlc;
 	tty->receive_room = 65536;
-	
+
 	/* change tty_io write() to not split large writes into 8K chunks */
 	set_bit(TTY_NO_WRITE_SPLIT,&tty->flags);
-	
+
 	/* flush receive data from driver */
 	tty_driver_flush_buffer(tty);
 
 	return 0;
-	
+
 }	/* end of n_tty_hdlc_open() */
 
 /**
@@ -269,9 +269,9 @@ static void n_hdlc_send_frames(struct n_hdlc *n_hdlc, struct tty_struct *tty)
 	unsigned long flags;
 	struct n_hdlc_buf *tbuf;
 
- check_again:
-		
- 	spin_lock_irqsave(&n_hdlc->tx_buf_list.spinlock, flags);
+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);
@@ -299,7 +299,7 @@ static void n_hdlc_send_frames(struct n_hdlc *n_hdlc, struct tty_struct *tty)
 		/* pretending it was accepted by driver */
 		if (actual < 0)
 			actual = tbuf->count;
-		
+
 		if (actual == tbuf->count) {
 			pr_debug("%s(%d)frame %p completed\n",
 					__FILE__, __LINE__, tbuf);
@@ -309,7 +309,7 @@ static void n_hdlc_send_frames(struct n_hdlc *n_hdlc, struct tty_struct *tty)
 
 			/* wait up sleeping writers */
 			wake_up_interruptible(&tty->write_wait);
-	
+
 			/* get next pending transmit buffer */
 			tbuf = n_hdlc_buf_get(&n_hdlc->tx_buf_list);
 		} else {
@@ -324,17 +324,17 @@ static void n_hdlc_send_frames(struct n_hdlc *n_hdlc, struct tty_struct *tty)
 			break;
 		}
 	}
-	
+
 	if (!tbuf)
 		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); 
-	
-        if (n_hdlc->woke_up)
-	  goto check_again;
+	spin_unlock_irqrestore(&n_hdlc->tx_buf_list.spinlock, flags);
+
+	if (n_hdlc->woke_up)
+		goto check_again;
 }	/* end of n_hdlc_send_frames() */
 
 /**
@@ -375,14 +375,14 @@ static void n_hdlc_tty_receive(struct tty_struct *tty, const __u8 *data,
 				__FILE__, __LINE__);
 		return;
 	}
-	
+
 	if ( count>maxframe ) {
 		pr_debug("%s(%d) rx count>maxframesize, data discarded\n",
 				__FILE__, __LINE__);
 		return;
 	}
 
-	/* get a free HDLC buffer */	
+	/* get a free HDLC buffer */
 	buf = n_hdlc_buf_get(&n_hdlc->rx_free_buf_list);
 	if (!buf) {
 		/* no buffers in free list, attempt to allocate another rx buffer */
@@ -391,20 +391,20 @@ static void n_hdlc_tty_receive(struct tty_struct *tty, const __u8 *data,
 			buf = kmalloc(struct_size(buf, buf, maxframe),
 				      GFP_ATOMIC);
 	}
-	
+
 	if (!buf) {
 		pr_debug("%s(%d) no more rx buffers, data discarded\n",
 				__FILE__, __LINE__);
 		return;
 	}
-		
+
 	/* copy received data to HDLC buffer */
 	memcpy(buf->buf,data,count);
 	buf->count=count;
 
 	/* add HDLC buffer to list of received frames */
 	n_hdlc_buf_put(&n_hdlc->rx_buf_list, buf);
-	
+
 	/* wake up any blocked reads and perform async signalling */
 	wake_up_interruptible (&tty->read_wait);
 	if (tty->fasync != NULL)
@@ -418,7 +418,7 @@ static void n_hdlc_tty_receive(struct tty_struct *tty, const __u8 *data,
  * @file - pointer to open file object
  * @buf - pointer to returned data buffer
  * @nr - size of returned data buffer
- * 	
+ *
  * Returns the number of bytes returned or error code.
  */
 static ssize_t n_hdlc_tty_read(struct tty_struct *tty, struct file *file,
@@ -468,7 +468,7 @@ static ssize_t n_hdlc_tty_read(struct tty_struct *tty, struct file *file,
 				n_hdlc_buf_put(&n_hdlc->rx_free_buf_list, rbuf);
 			break;
 		}
-			
+
 		/* no data */
 		if (tty_io_nonblock(tty, file)) {
 			ret = -EAGAIN;
@@ -487,7 +487,7 @@ static ssize_t n_hdlc_tty_read(struct tty_struct *tty, struct file *file,
 	__set_current_state(TASK_RUNNING);
 
 	return ret;
-	
+
 }	/* end of n_hdlc_tty_read() */
 
 /**
@@ -496,7 +496,7 @@ static ssize_t n_hdlc_tty_read(struct tty_struct *tty, struct file *file,
  * @file - pointer to file object data
  * @data - pointer to transmit data (one frame)
  * @count - size of transmit frame in bytes
- * 		
+ *
  * Returns the number of bytes written (or error code).
  */
 static ssize_t n_hdlc_tty_write(struct tty_struct *tty, struct file *file,
@@ -519,12 +519,12 @@ static ssize_t n_hdlc_tty_write(struct tty_struct *tty, struct file *file,
 				__func__, count, maxframe);
 		count = maxframe;
 	}
-	
+
 	add_wait_queue(&tty->write_wait, &wait);
 
 	for (;;) {
 		set_current_state(TASK_INTERRUPTIBLE);
-	
+
 		tbuf = n_hdlc_buf_get(&n_hdlc->tx_free_buf_list);
 		if (tbuf)
 			break;
@@ -544,7 +544,7 @@ static ssize_t n_hdlc_tty_write(struct tty_struct *tty, struct file *file,
 	__set_current_state(TASK_RUNNING);
 	remove_wait_queue(&tty->write_wait, &wait);
 
-	if (!error) {		
+	if (!error) {
 		/* Retrieve the user's buffer */
 		memcpy(tbuf->buf, data, count);
 
@@ -555,7 +555,7 @@ static ssize_t n_hdlc_tty_write(struct tty_struct *tty, struct file *file,
 	}
 
 	return error;
-	
+
 }	/* end of n_hdlc_tty_write() */
 
 /**
@@ -623,7 +623,7 @@ static int n_hdlc_tty_ioctl(struct tty_struct *tty, struct file *file,
 		break;
 	}
 	return error;
-	
+
 }	/* end of n_hdlc_tty_ioctl() */
 
 /**
@@ -631,7 +631,7 @@ static int n_hdlc_tty_ioctl(struct tty_struct *tty, struct file *file,
  * @tty - pointer to tty instance data
  * @filp - pointer to open file object for device
  * @poll_table - wait queue for operations
- * 
+ *
  * Determine which operations (read/write) will not block and return info
  * to caller.
  * Returns a bit mask containing info on which ops will not block.
@@ -712,7 +712,7 @@ static struct n_hdlc *n_hdlc_alloc(void)
 	n_hdlc->magic  = HDLC_MAGIC;
 
 	return n_hdlc;
-	
+
 }	/* end of n_hdlc_alloc() */
 
 /**
@@ -754,7 +754,7 @@ static void n_hdlc_buf_put(struct n_hdlc_buf_list *buf_list,
 /**
  * n_hdlc_buf_get - remove and return an HDLC buffer from list
  * @buf_list - pointer to HDLC buffer list
- * 
+ *
  * Remove and return an HDLC buffer from the head of the specified HDLC buffer
  * list.
  * Returns a pointer to HDLC buffer if available, otherwise %NULL.
@@ -808,7 +808,7 @@ static int __init n_hdlc_init(void)
 				status);
 
 	return status;
-	
+
 }	/* end of init_module() */
 
 static void __exit n_hdlc_exit(void)
-- 
2.25.0

[PATCH 21/24] n_hdlc: remove spaces between function name and (

[Jiri Slaby]

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/n_hdlc.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/n_hdlc.c b/drivers/tty/n_hdlc.c
index ee27573b0624..d52bcfa7f8e4 100644
--- a/drivers/tty/n_hdlc.c
+++ b/drivers/tty/n_hdlc.c
@@ -152,7 +152,7 @@ static struct n_hdlc_buf *n_hdlc_buf_get(struct n_hdlc_buf_list *list);
 
 /* Local functions */
 
-static struct n_hdlc *n_hdlc_alloc (void);
+static struct n_hdlc *n_hdlc_alloc(void);
 
 /* max frame size for memory allocations */
 static int maxframe = 4096;
@@ -222,7 +222,7 @@ static void n_hdlc_tty_close(struct tty_struct *tty)
  *
  * Returns 0 if success, otherwise error code
  */
-static int n_hdlc_tty_open (struct tty_struct *tty)
+static int n_hdlc_tty_open(struct tty_struct *tty)
 {
 	struct n_hdlc *n_hdlc = tty->disc_data;
 
@@ -347,7 +347,7 @@ static void n_hdlc_tty_wakeup(struct tty_struct *tty)
 {
 	struct n_hdlc *n_hdlc = tty->disc_data;
 
-	n_hdlc_send_frames (n_hdlc, tty);
+	n_hdlc_send_frames(n_hdlc, tty);
 }	/* end of n_hdlc_tty_wakeup() */
 
 /**
@@ -406,7 +406,7 @@ static void n_hdlc_tty_receive(struct tty_struct *tty, const __u8 *data,
 	n_hdlc_buf_put(&n_hdlc->rx_buf_list, buf);
 
 	/* wake up any blocked reads and perform async signalling */
-	wake_up_interruptible (&tty->read_wait);
+	wake_up_interruptible(&tty->read_wait);
 	if (tty->fasync != NULL)
 		kill_fasync(&tty->fasync, SIGIO, POLL_IN);
 
-- 
2.25.0

[PATCH 22/24] n_hdlc: add missing spaces after commas

[Jiri Slaby]

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/n_hdlc.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/tty/n_hdlc.c b/drivers/tty/n_hdlc.c
index d52bcfa7f8e4..7ae4347a946f 100644
--- a/drivers/tty/n_hdlc.c
+++ b/drivers/tty/n_hdlc.c
@@ -201,7 +201,7 @@ static void n_hdlc_tty_close(struct tty_struct *tty)
 		return;
 	}
 #if defined(TTY_NO_WRITE_SPLIT)
-	clear_bit(TTY_NO_WRITE_SPLIT,&tty->flags);
+	clear_bit(TTY_NO_WRITE_SPLIT, &tty->flags);
 #endif
 	tty->disc_data = NULL;
 
@@ -245,7 +245,7 @@ static int n_hdlc_tty_open(struct tty_struct *tty)
 	tty->receive_room = 65536;
 
 	/* change tty_io write() to not split large writes into 8K chunks */
-	set_bit(TTY_NO_WRITE_SPLIT,&tty->flags);
+	set_bit(TTY_NO_WRITE_SPLIT, &tty->flags);
 
 	/* flush receive data from driver */
 	tty_driver_flush_buffer(tty);
@@ -399,7 +399,7 @@ static void n_hdlc_tty_receive(struct tty_struct *tty, const __u8 *data,
 	}
 
 	/* copy received data to HDLC buffer */
-	memcpy(buf->buf,data,count);
+	memcpy(buf->buf, data, count);
 	buf->count=count;
 
 	/* add HDLC buffer to list of received frames */
@@ -550,8 +550,8 @@ static ssize_t n_hdlc_tty_write(struct tty_struct *tty, struct file *file,
 
 		/* Send the data */
 		tbuf->count = error = count;
-		n_hdlc_buf_put(&n_hdlc->tx_buf_list,tbuf);
-		n_hdlc_send_frames(n_hdlc,tty);
+		n_hdlc_buf_put(&n_hdlc->tx_buf_list, tbuf);
+		n_hdlc_send_frames(n_hdlc, tty);
 	}
 
 	return error;
@@ -586,14 +586,14 @@ static int n_hdlc_tty_ioctl(struct tty_struct *tty, struct file *file,
 	case FIONREAD:
 		/* report count of read data available */
 		/* in next available frame (if any) */
-		spin_lock_irqsave(&n_hdlc->rx_buf_list.spinlock,flags);
+		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);
+		spin_unlock_irqrestore(&n_hdlc->rx_buf_list.spinlock, flags);
 		error = put_user(count, (int __user *)arg);
 		break;
 
@@ -601,12 +601,12 @@ static int n_hdlc_tty_ioctl(struct tty_struct *tty, struct file *file,
 		/* 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);
+		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);
+		spin_unlock_irqrestore(&n_hdlc->tx_buf_list.spinlock, flags);
 		error = put_user(count, (int __user *)arg);
 		break;
 
-- 
2.25.0

[PATCH 23/24] n_hdlc: fix whitespace around binary operators

[Jiri Slaby]

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/n_hdlc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/n_hdlc.c b/drivers/tty/n_hdlc.c
index 7ae4347a946f..b74a8ecc65b5 100644
--- a/drivers/tty/n_hdlc.c
+++ b/drivers/tty/n_hdlc.c
@@ -376,7 +376,7 @@ static void n_hdlc_tty_receive(struct tty_struct *tty, const __u8 *data,
 		return;
 	}
 
-	if ( count>maxframe ) {
+	if (count > maxframe) {
 		pr_debug("%s(%d) rx count>maxframesize, data discarded\n",
 				__FILE__, __LINE__);
 		return;
@@ -400,7 +400,7 @@ static void n_hdlc_tty_receive(struct tty_struct *tty, const __u8 *data,
 
 	/* copy received data to HDLC buffer */
 	memcpy(buf->buf, data, count);
-	buf->count=count;
+	buf->count = count;
 
 	/* add HDLC buffer to list of received frames */
 	n_hdlc_buf_put(&n_hdlc->rx_buf_list, buf);
@@ -514,7 +514,7 @@ static ssize_t n_hdlc_tty_write(struct tty_struct *tty, struct file *file,
 		return -EIO;
 
 	/* verify frame size */
-	if (count > maxframe ) {
+	if (count > maxframe) {
 		pr_debug("%s: truncating user packet from %zu to %d\n",
 				__func__, count, maxframe);
 		count = maxframe;
-- 
2.25.0

[PATCH 24/24] n_hdlc: wrap a comment properly

[Jiri Slaby]

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/n_hdlc.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/n_hdlc.c b/drivers/tty/n_hdlc.c
index b74a8ecc65b5..cd1319d26c45 100644
--- a/drivers/tty/n_hdlc.c
+++ b/drivers/tty/n_hdlc.c
@@ -385,8 +385,10 @@ static void n_hdlc_tty_receive(struct tty_struct *tty, const __u8 *data,
 	/* get a free HDLC buffer */
 	buf = n_hdlc_buf_get(&n_hdlc->rx_free_buf_list);
 	if (!buf) {
-		/* no buffers in free list, attempt to allocate another rx buffer */
-		/* unless the maximum count has been reached */
+		/*
+		 * no buffers in free list, attempt to allocate another rx
+		 * buffer unless the maximum count has been reached
+		 */
 		if (n_hdlc->rx_buf_list.count < MAX_RX_BUF_COUNT)
 			buf = kmalloc(struct_size(buf, buf, maxframe),
 				      GFP_ATOMIC);
-- 
2.25.0

Re: [PATCH 03/24] n_hdlc: convert debuglevel use to pr_debug

[Joe Perches]

On Wed, 2020-02-19 at 09:40 +0100, Jiri Slaby wrote:
> With pr_debug we have a fine-grained control about debugging prints. So
> convert the use of global debuglevel variable and tests to a commonly
> used pr_debug. And drop debuglevel completely.
> 
> This also implicitly adds a loglevel to the messages (KERN_DEBUG) as it
> was missing on most of them.
[]
> diff --git a/drivers/tty/n_hdlc.c b/drivers/tty/n_hdlc.c
[]
> @@ -310,11 +306,9 @@ static int n_hdlc_tty_open (struct tty_struct *tty)
>  {
>  	struct n_hdlc *n_hdlc = tty2n_hdlc (tty);
>  
> -	if (debuglevel >= DEBUG_LEVEL_INFO)	
> -		printk("%s(%d)n_hdlc_tty_open() called (device=%s)\n",
> -		__FILE__,__LINE__,
> -		tty->name);
> -		
> +	pr_debug("%s(%d)%s() called (device=%s)\n",
> +			__FILE__, __LINE__, __func__, tty->name);

Perhaps remove all the __FILE__ and __LINE__ arguments as
dynamic debug could emit module and __LINE__ when necessary.

Re: [PATCH 03/24] n_hdlc: convert debuglevel use to pr_debug

[Greg KH]

On Wed, Feb 19, 2020 at 04:20:33AM -0800, Joe Perches wrote:
> On Wed, 2020-02-19 at 09:40 +0100, Jiri Slaby wrote:
> > With pr_debug we have a fine-grained control about debugging prints. So
> > convert the use of global debuglevel variable and tests to a commonly
> > used pr_debug. And drop debuglevel completely.
> > 
> > This also implicitly adds a loglevel to the messages (KERN_DEBUG) as it
> > was missing on most of them.
> []
> > diff --git a/drivers/tty/n_hdlc.c b/drivers/tty/n_hdlc.c
> []
> > @@ -310,11 +306,9 @@ static int n_hdlc_tty_open (struct tty_struct *tty)
> >  {
> >  	struct n_hdlc *n_hdlc = tty2n_hdlc (tty);
> >  
> > -	if (debuglevel >= DEBUG_LEVEL_INFO)	
> > -		printk("%s(%d)n_hdlc_tty_open() called (device=%s)\n",
> > -		__FILE__,__LINE__,
> > -		tty->name);
> > -		
> > +	pr_debug("%s(%d)%s() called (device=%s)\n",
> > +			__FILE__, __LINE__, __func__, tty->name);
> 
> Perhaps remove all the __FILE__ and __LINE__ arguments as
> dynamic debug could emit module and __LINE__ when necessary.

That can be a set of follow-on patches.  This patch is fine as-is for
now.

thanks,

greg k-h

Re: [PATCH 03/24] n_hdlc: convert debuglevel use to pr_debug

[Jiri Slaby]

On 19. 02. 20, 13:20, Joe Perches wrote:
> On Wed, 2020-02-19 at 09:40 +0100, Jiri Slaby wrote:
>> With pr_debug we have a fine-grained control about debugging prints. So
>> convert the use of global debuglevel variable and tests to a commonly
>> used pr_debug. And drop debuglevel completely.
>>
>> This also implicitly adds a loglevel to the messages (KERN_DEBUG) as it
>> was missing on most of them.
> []
>> diff --git a/drivers/tty/n_hdlc.c b/drivers/tty/n_hdlc.c
> []
>> @@ -310,11 +306,9 @@ static int n_hdlc_tty_open (struct tty_struct *tty)
>>  {
>>  	struct n_hdlc *n_hdlc = tty2n_hdlc (tty);
>>  
>> -	if (debuglevel >= DEBUG_LEVEL_INFO)	
>> -		printk("%s(%d)n_hdlc_tty_open() called (device=%s)\n",
>> -		__FILE__,__LINE__,
>> -		tty->name);
>> -		
>> +	pr_debug("%s(%d)%s() called (device=%s)\n",
>> +			__FILE__, __LINE__, __func__, tty->name);
> 
> Perhaps remove all the __FILE__ and __LINE__ arguments as
> dynamic debug could emit module and __LINE__ when necessary.

Noted for later.

thanks,
-- 
js
suse labs