linux-serial.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-serial@vger.kernel.org,
	Dmitry Safonov <0x7f454c46@gmail.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Subject: [PATCH v2 1/4] serial: core: Use string length for SysRq magic sequence
Date: Tue, 10 Mar 2020 19:43:34 +0200	[thread overview]
Message-ID: <20200310174337.74109-1-andriy.shevchenko@linux.intel.com> (raw)

Compiler is not happy about using ARRAY_SIZE() in comparison to smaller type:

  CC      drivers/tty/serial/serial_core.o
.../serial_core.c: In function ‘uart_try_toggle_sysrq’:
.../serial_core.c:3222:24: warning: comparison is always false due to limited range of data type [-Wtype-limits]
 3222 |  if (++port->sysrq_seq < (ARRAY_SIZE(sysrq_toggle_seq) - 1)) {
      |                        ^

Looking at the code it appears that there is an additional weirdness,
i.e. use ARRAY_SIZE() against simple string literal. Yes, the idea probably
was to allow '\0' in the sequence, but it's impractical: kernel configuration
won't accept it to begin with followed by a comment about '\0' before
comparison in question.

Drop all these by switching to strlen() and convert code accordingly.

Note, GCC seems clever enough to calculate string length at compile time.

Fixes: 68af43173d3f ("serial/sysrq: Add MAGIC_SYSRQ_SERIAL_SEQUENCE")
Reviewed-by: Dmitry Safonov <0x7f454c46@gmail.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v2: add note that GCC optimizes strlen() at run time, add tag
 drivers/tty/serial/serial_core.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index aec98db45406..ec3b833e9f22 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -3209,7 +3209,9 @@ static DECLARE_WORK(sysrq_enable_work, uart_sysrq_on);
  */
 static bool uart_try_toggle_sysrq(struct uart_port *port, unsigned int ch)
 {
-	if (ARRAY_SIZE(sysrq_toggle_seq) <= 1)
+	int sysrq_toggle_seq_len = strlen(sysrq_toggle_seq);
+
+	if (!sysrq_toggle_seq_len)
 		return false;
 
 	BUILD_BUG_ON(ARRAY_SIZE(sysrq_toggle_seq) >= U8_MAX);
@@ -3218,8 +3220,7 @@ static bool uart_try_toggle_sysrq(struct uart_port *port, unsigned int ch)
 		return false;
 	}
 
-	/* Without the last \0 */
-	if (++port->sysrq_seq < (ARRAY_SIZE(sysrq_toggle_seq) - 1)) {
+	if (++port->sysrq_seq < sysrq_toggle_seq_len) {
 		port->sysrq = jiffies + SYSRQ_TIMEOUT;
 		return true;
 	}
-- 
2.25.1


             reply	other threads:[~2020-03-10 17:43 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-10 17:43 Andy Shevchenko [this message]
2020-03-10 17:43 ` [PATCH v2 2/4] serial: core: Print escaped SysRq Magic sequence if enabled Andy Shevchenko
2020-03-10 17:43 ` [PATCH v2 3/4] serial: core: Use uart_console() helper in SysRq code Andy Shevchenko
2020-03-10 17:43 ` [PATCH v2 4/4] serial: core: Refactor uart_unlock_and_check_sysrq() Andy Shevchenko
2020-03-10 17:45   ` Dmitry Safonov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200310174337.74109-1-andriy.shevchenko@linux.intel.com \
    --to=andriy.shevchenko@linux.intel.com \
    --cc=0x7f454c46@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-serial@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).