linux-kernel - Re: [PATCH][KJ] 8250: remove unnecessary variable tmout from wait_for

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <x7hcim4ph1.fsf@saeurebad.de>
Date:	Thu, 13 Dec 2007 15:51:54 +0100
From:	Johannes Weiner <hannes-kernel@...urebad.de>
To:	Andre Haupt <andre@...wigglers.org>
Cc:	kernel-janitors@...r.kernel.org, linux-serial@...r.kernel.org,
	linux-kernel@...r.kernel.org, trivial@...nel.org
Subject: Re: [PATCH][KJ] 8250: remove unnecessary variable tmout from  wait_for_xmitr()

Hi,

Andre Haupt <andre@...wigglers.org> writes:

> This fixes a sparse warning about symbol tmout shadowing an earlier one.
>
> Signed-off-by: Andre Haupt <andre@...wigglers.org>
> ---
>  drivers/serial/8250.c |    1 -
>  1 files changed, 0 insertions(+), 1 deletions(-)

The whole timeout loops look somewhat odd.  How about the following,
totally untested, solution?

This converts the function to use more common timeout-loop constructs,
gets rid of the shadowed variable and localises another variable even
more! Hooray!

	Hannes

Signed-off-by: Johannes Weiner <hannes@...urebad.de>
---

 drivers/serial/8250.c |   18 +++++++++---------
 1 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c
index f94109c..7215f6a 100644
--- a/drivers/serial/8250.c
+++ b/drivers/serial/8250.c
@@ -1712,30 +1712,30 @@ static void serial8250_break_ctl(struct uart_port *port, int break_state)
  */
 static inline void wait_for_xmitr(struct uart_8250_port *up, int bits)
 {
-	unsigned int status, tmout = 10000;
+	unsigned int tmout;
 
 	/* Wait up to 10ms for the character(s) to be sent. */
+	tmout = jiffies + jiffies_to_msecs(10);
 	do {
-		status = serial_in(up, UART_LSR);
+		unsigned int status = serial_in(up, UART_LSR);
 
-		up->lsr_saved_flags |= status & LSR_SAVE_FLAGS;
-
-		if (--tmout == 0)
+		up->lsr_saved_flags |= (status & LSR_SAVE_FLAGS);
+		if ((status & bits) == bits)
 			break;
 		udelay(1);
-	} while ((status & bits) != bits);
+	} while (time_after_eq(tmout, jiffies));
 
 	/* Wait up to 1s for flow control if necessary */
 	if (up->port.flags & UPF_CONS_FLOW) {
-		unsigned int tmout;
-		for (tmout = 1000000; tmout; tmout--) {
+		tmout = jiffies + jiffies_to_msecs(1000);
+		do {
 			unsigned int msr = serial_in(up, UART_MSR);
 			up->msr_saved_flags |= msr & MSR_SAVE_FLAGS;
 			if (msr & UART_MSR_CTS)
 				break;
 			udelay(1);
 			touch_nmi_watchdog();
-		}
+		} while (time_after_eq(tmout, jiffies));
 	}
 }
 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/