On Tue, Mar 10, 2020 at 03:57:17PM +0000, Dmitry Safonov wrote:
> Hi Andy,
>
> On 3/10/20 2:57 PM, Andy Shevchenko wrote:
> > On Tue, Mar 10, 2020 at 02:38:48PM +0000, Dmitry Safonov wrote:
> >> On 3/10/20 1:20 PM, Andy Shevchenko wrote:
> >> [..]> @@ -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;
> >>
> >> Eh, I wanted to avoid the strlen() call in runtime for every time sysrq
> >> is pressed. It's not very frequent moment surely, but..
> >
> > I really don't like ARRAY_SIZE() against plain strings.
> > This will use \0 inclusively and confuse the understanding the code.
>
> I still feel a bit awkward that handler will run strlen() for something
> that could be done compile-time, but I won't insist.
Have you seen to the assembly?
For me it seems that GCC is clever enough to precalc length for constant
literals.
With default string (empty) there is no uart_sysrq_on() at all in the code.
With "pqr" I see this function and in particular:
621: be 03 00 00 00 mov $0x3,%esi
With "pqrst"
621: be 05 00 00 00 mov $0x5,%esi
(Note, I compiled entire series, that's why uart_sysrq_on() has strlen() in)
> Thanks again for looking into warning,
You are welcome.
--
With Best Regards,
Andy Shevchenko
