[04/15] pcmcia: synclink_cs: remove dead paranoia_check, warn for missing line

Message ID 051083d29e5812608deb034dfa86ae0c583fee44.1666822928.git.nabijaczleweli@nabijaczleweli.xyz
State New
Headers
Series [01/15] hamradio: baycom: remove BAYCOM_MAGIC |

Commit Message

Ahelenia Ziemiańska Oct. 26, 2022, 10:42 p.m. UTC
  MGSLPC_PARANOIA_CHECK has never been defined automatically,
and devices with null driver_info can't happen, since we reject the open
in that case

Move the log statement from dead code to the check,
and log the state inconsistency like we do above for the line count
("invalid line #%d.")

Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
---
 drivers/char/pcmcia/synclink_cs.c | 71 +++----------------------------
 1 file changed, 6 insertions(+), 65 deletions(-)
  

Comments

Andy Shevchenko Oct. 27, 2022, 4:53 a.m. UTC | #1
On Thu, Oct 27, 2022 at 1:42 AM наб <nabijaczleweli@nabijaczleweli.xyz> wrote:
>
> MGSLPC_PARANOIA_CHECK has never been defined automatically,
> and devices with null driver_info can't happen, since we reject the open
> in that case
>
> Move the log statement from dead code to the check,
> and log the state inconsistency like we do above for the line count
> ("invalid line #%d.")

Please, respect English grammar and punctuation, in particular don't
forget the trailing periods.

...

> +       if (!info) {
> +               printk(KERN_WARNING "%s(%d):mgslpc_open: "
> +                       "no device for line #%d.\n",
> +                       __FILE__, __LINE__, line);

Even though the old code uses this awkward style for messages, please
follow the modern one, i.e.

               pr_warn("mgslpc_open: no device for line #%d.\n", line);

>                 return -ENODEV;
> +       }
  
Ahelenia Ziemiańska Oct. 27, 2022, 11:31 a.m. UTC | #2
On Thu, Oct 27, 2022 at 07:53:34AM +0300, Andy Shevchenko wrote:
> On Thu, Oct 27, 2022 at 1:42 AM наб <nabijaczleweli@nabijaczleweli.xyz> wrote:
> > in that case
> > ("invalid line #%d.")
> 
> Please, respect English grammar and punctuation, in particular don't
> forget the trailing periods.
> 
> > +       if (!info) {
> > +               printk(KERN_WARNING "%s(%d):mgslpc_open: "
> > +                       "no device for line #%d.\n",
> > +                       __FILE__, __LINE__, line);
> 
> Even though the old code uses this awkward style for messages, please
> follow the modern one, i.e.
> 
>                pr_warn("mgslpc_open: no device for line #%d.\n", line);

Both applied for v2.

наб
  

Patch

diff --git a/drivers/char/pcmcia/synclink_cs.c b/drivers/char/pcmcia/synclink_cs.c
index 262f087bfc01..19b6118639b4 100644
--- a/drivers/char/pcmcia/synclink_cs.c
+++ b/drivers/char/pcmcia/synclink_cs.c
@@ -644,25 +644,6 @@  static int mgslpc_resume(struct pcmcia_device *link)
 }
 
 
-static inline bool mgslpc_paranoia_check(MGSLPC_INFO *info,
-					char *name, const char *routine)
-{
-#ifdef MGSLPC_PARANOIA_CHECK
-	static const char *badinfo =
-		"Warning: null mgslpc_info for (%s) in %s\n";
-
-	if (!info) {
-		printk(badinfo, name, routine);
-		return true;
-	}
-#else
-	if (!info)
-		return true;
-#endif
-	return false;
-}
-
-
 #define CMD_RXFIFO      BIT7	// release current rx FIFO
 #define CMD_RXRESET     BIT6	// receiver reset
 #define CMD_RXFIFO_READ BIT5
@@ -694,8 +675,6 @@  static void tx_pause(struct tty_struct *tty)
 	MGSLPC_INFO *info = (MGSLPC_INFO *)tty->driver_data;
 	unsigned long flags;
 
-	if (mgslpc_paranoia_check(info, tty->name, "tx_pause"))
-		return;
 	if (debug_level >= DEBUG_LEVEL_INFO)
 		printk("tx_pause(%s)\n", info->device_name);
 
@@ -710,8 +689,6 @@  static void tx_release(struct tty_struct *tty)
 	MGSLPC_INFO *info = (MGSLPC_INFO *)tty->driver_data;
 	unsigned long flags;
 
-	if (mgslpc_paranoia_check(info, tty->name, "tx_release"))
-		return;
 	if (debug_level >= DEBUG_LEVEL_INFO)
 		printk("tx_release(%s)\n", info->device_name);
 
@@ -1476,9 +1453,6 @@  static int mgslpc_put_char(struct tty_struct *tty, unsigned char ch)
 			__FILE__, __LINE__, ch, info->device_name);
 	}
 
-	if (mgslpc_paranoia_check(info, tty->name, "mgslpc_put_char"))
-		return 0;
-
 	if (!info->tx_buf)
 		return 0;
 
@@ -1508,9 +1482,6 @@  static void mgslpc_flush_chars(struct tty_struct *tty)
 		printk("%s(%d):mgslpc_flush_chars() entry on %s tx_count=%d\n",
 			__FILE__, __LINE__, info->device_name, info->tx_count);
 
-	if (mgslpc_paranoia_check(info, tty->name, "mgslpc_flush_chars"))
-		return;
-
 	if (info->tx_count <= 0 || tty->flow.stopped ||
 	    tty->hw_stopped || !info->tx_buf)
 		return;
@@ -1546,8 +1517,7 @@  static int mgslpc_write(struct tty_struct * tty,
 		printk("%s(%d):mgslpc_write(%s) count=%d\n",
 			__FILE__, __LINE__, info->device_name, count);
 
-	if (mgslpc_paranoia_check(info, tty->name, "mgslpc_write") ||
-		!info->tx_buf)
+	if (!info->tx_buf)
 		goto cleanup;
 
 	if (info->params.mode == MGSL_MODE_HDLC) {
@@ -1600,9 +1570,6 @@  static unsigned int mgslpc_write_room(struct tty_struct *tty)
 	MGSLPC_INFO *info = (MGSLPC_INFO *)tty->driver_data;
 	int ret;
 
-	if (mgslpc_paranoia_check(info, tty->name, "mgslpc_write_room"))
-		return 0;
-
 	if (info->params.mode == MGSL_MODE_HDLC) {
 		/* HDLC (frame oriented) mode */
 		if (info->tx_active)
@@ -1632,9 +1599,6 @@  static unsigned int mgslpc_chars_in_buffer(struct tty_struct *tty)
 		printk("%s(%d):mgslpc_chars_in_buffer(%s)\n",
 			 __FILE__, __LINE__, info->device_name);
 
-	if (mgslpc_paranoia_check(info, tty->name, "mgslpc_chars_in_buffer"))
-		return 0;
-
 	if (info->params.mode == MGSL_MODE_HDLC)
 		rc = info->tx_active ? info->max_frame_size : 0;
 	else
@@ -1658,9 +1622,6 @@  static void mgslpc_flush_buffer(struct tty_struct *tty)
 		printk("%s(%d):mgslpc_flush_buffer(%s) entry\n",
 			 __FILE__, __LINE__, info->device_name);
 
-	if (mgslpc_paranoia_check(info, tty->name, "mgslpc_flush_buffer"))
-		return;
-
 	spin_lock_irqsave(&info->lock, flags);
 	info->tx_count = info->tx_put = info->tx_get = 0;
 	del_timer(&info->tx_timer);
@@ -1681,9 +1642,6 @@  static void mgslpc_send_xchar(struct tty_struct *tty, char ch)
 		printk("%s(%d):mgslpc_send_xchar(%s,%d)\n",
 			 __FILE__, __LINE__, info->device_name, ch);
 
-	if (mgslpc_paranoia_check(info, tty->name, "mgslpc_send_xchar"))
-		return;
-
 	info->x_char = ch;
 	if (ch) {
 		spin_lock_irqsave(&info->lock, flags);
@@ -1704,9 +1662,6 @@  static void mgslpc_throttle(struct tty_struct * tty)
 		printk("%s(%d):mgslpc_throttle(%s) entry\n",
 			 __FILE__, __LINE__, info->device_name);
 
-	if (mgslpc_paranoia_check(info, tty->name, "mgslpc_throttle"))
-		return;
-
 	if (I_IXOFF(tty))
 		mgslpc_send_xchar(tty, STOP_CHAR(tty));
 
@@ -1729,9 +1684,6 @@  static void mgslpc_unthrottle(struct tty_struct * tty)
 		printk("%s(%d):mgslpc_unthrottle(%s) entry\n",
 			 __FILE__, __LINE__, info->device_name);
 
-	if (mgslpc_paranoia_check(info, tty->name, "mgslpc_unthrottle"))
-		return;
-
 	if (I_IXOFF(tty)) {
 		if (info->x_char)
 			info->x_char = 0;
@@ -2160,9 +2112,6 @@  static int mgslpc_break(struct tty_struct *tty, int break_state)
 		printk("%s(%d):mgslpc_break(%s,%d)\n",
 			 __FILE__, __LINE__, info->device_name, break_state);
 
-	if (mgslpc_paranoia_check(info, tty->name, "mgslpc_break"))
-		return -EINVAL;
-
 	spin_lock_irqsave(&info->lock, flags);
 	if (break_state == -1)
 		set_reg_bits(info, CHA+DAFO, BIT6);
@@ -2218,9 +2167,6 @@  static int mgslpc_ioctl(struct tty_struct *tty,
 		printk("%s(%d):mgslpc_ioctl %s cmd=%08X\n", __FILE__, __LINE__,
 			info->device_name, cmd);
 
-	if (mgslpc_paranoia_check(info, tty->name, "mgslpc_ioctl"))
-		return -ENODEV;
-
 	if (cmd != TIOCMIWAIT) {
 		if (tty_io_error(tty))
 		    return -EIO;
@@ -2312,9 +2258,6 @@  static void mgslpc_close(struct tty_struct *tty, struct file * filp)
 	MGSLPC_INFO * info = (MGSLPC_INFO *)tty->driver_data;
 	struct tty_port *port = &info->port;
 
-	if (mgslpc_paranoia_check(info, tty->name, "mgslpc_close"))
-		return;
-
 	if (debug_level >= DEBUG_LEVEL_INFO)
 		printk("%s(%d):mgslpc_close(%s) entry, count=%d\n",
 			 __FILE__, __LINE__, info->device_name, port->count);
@@ -2352,9 +2295,6 @@  static void mgslpc_wait_until_sent(struct tty_struct *tty, int timeout)
 		printk("%s(%d):mgslpc_wait_until_sent(%s) entry\n",
 			 __FILE__, __LINE__, info->device_name);
 
-	if (mgslpc_paranoia_check(info, tty->name, "mgslpc_wait_until_sent"))
-		return;
-
 	if (!tty_port_initialized(&info->port))
 		goto exit;
 
@@ -2412,9 +2352,6 @@  static void mgslpc_hangup(struct tty_struct *tty)
 		printk("%s(%d):mgslpc_hangup(%s)\n",
 			 __FILE__, __LINE__, info->device_name);
 
-	if (mgslpc_paranoia_check(info, tty->name, "mgslpc_hangup"))
-		return;
-
 	mgslpc_flush_buffer(tty);
 	shutdown(info, tty);
 	tty_port_hangup(&info->port);
@@ -2468,8 +2405,12 @@  static int mgslpc_open(struct tty_struct *tty, struct file * filp)
 	info = mgslpc_device_list;
 	while(info && info->line != line)
 		info = info->next_device;
-	if (mgslpc_paranoia_check(info, tty->name, "mgslpc_open"))
+	if (!info) {
+		printk(KERN_WARNING "%s(%d):mgslpc_open: "
+			"no device for line #%d.\n",
+			__FILE__, __LINE__, line);
 		return -ENODEV;
+	}
 
 	port = &info->port;
 	tty->driver_data = info;