linux-kernel - Re: [PATCH v6] can, tty: can327 CAN/ldisc driver for ELM327 based OBD-II adapters

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMZ6Rq+jQA5MhSG-W_h0s8nJUZVKHxYsp93vEftz9DDRgb86KA@mail.gmail.com>
Date:   Sat, 14 May 2022 21:24:45 +0900
From:   Vincent Mailhol <vincent.mailhol@...il.com>
To:     Max Staudt <max@...as.org>
Cc:     Wolfgang Grandegger <wg@...ndegger.com>,
        Marc Kleine-Budde <mkl@...gutronix.de>,
        linux-can@...r.kernel.org,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Oliver Neukum <oneukum@...e.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v6] can, tty: can327 CAN/ldisc driver for ELM327 based
 OBD-II adapters

On Tue. 14 May 2022 à 20:11, Max Staudt <max@...as.org> wrote:
> On Sat, 14 May 2022 12:14:24 +0900
> Vincent Mailhol <vincent.mailhol@...il.com> wrote:
>
> > But I still think it is possible to do pointer arithmetic.
> >
> > len = strnchr(elm->rxbuf, elm->rxfill, '\r') - elm->rxbuf;
> > (I let you check that I did not do an off by one mistake).
> >
> > The above should also work with memchr(). Although the C standard
> > doesn't allow pointer arithmetic on void *, GNU C adds an extension
> > for that: https://gcc.gnu.org/onlinedocs/gcc/Pointer-Arith.html
> >
> > As I said before, your for loop is not fundamentally wrong, this is
> > just not my prefered approach. You have my suggestion, choose what you
> > prefer.
>
> Yeah, this is the arithmetic that I'd like to avoid here. In my
> opinion, it is clearer with indices.
>
> If I were searching through a UTF-8 string (i.e. with variable width
> chars), that'd be another matter entirely IMHO, and I'd rely on C
> library functions that know more about UTF that I do. But it's really
> just naked ASCII bytes this time.
>
>
> ...unless memchr() may be faster than the loop? Could this happen?

It depends on many factors, the length of the string, the architecture
on which you compile it, whether the compiler decides to inline it or
not…

For long strings, there are some tricks to scan through the memory
using 64 bits operation instead of doing it bytes per bytes. But there
is a preparation overhead which makes it not worth it for small
buffers.

You can look at glibc implementation for reference:
https://github.com/lattera/glibc/blob/master/string/memchr.c

Of course, the kernel does not use glibc but often compilers
__builtin_memchr() instead (one more time, it depends on the
architecture). But it will give you an idea of why memchr() can be
faster than the loop.

Finally, the compiler might also detect your loop and take the
initiative to replace it with a call to memchr(). You have to check
the assembly to see if this is the case.

For a device which speaks on UART which by nature is slow,
optimizations are not critical, so readability is the priority, and is
why I prefer using the library functions.