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: <CAMZ6RqLj2eLX2UWMvGc9rH2SP6HNuqBAXnwJ6q6qvk+7QWE8pA@mail.gmail.com>
Date:   Sat, 14 May 2022 12:14:24 +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 Sat. 14 mai 2022 at 03:59, Max Staudt <max@...as.org> wrote:
> On Fri, 13 May 2022 15:31:20 +0900
> Vincent Mailhol <vincent.mailhol@...il.com> wrote:
> > On Fri. 13 May 2022 at 11:38, Vincent Mailhol
> > <vincent.mailhol@...il.com> wrote: [...]
> > > > +       case ELM327_STATE_RECEIVING:
> > > > +               /* Find <CR> delimiting feedback lines. */
> > > > +               for (len = 0;
> > > > +                    (len < elm->rxfill) && (elm->rxbuf[len] !=
> > > > '\r');
> > > > +                    len++) {
> > > > +                       /* empty loop */
> > >
> > > Question of taste but would prefer a while look with the len++ in
> > > the body (if you prefer to do as above, no need to argue, just keep
> > > it like it is).
> >
> > Actually, what about this?
> >
> > len = strnchr(elm->rxbuf, elm->rxfill, '\r');
>
> Actually I'd use memchr() if anything, but not really here. I do end up
> using the actual index. And since both strchr() and mrmchr() return
> pointers, I'd rather avoid them because I prefer to use indices
> whenever possible.

You are right. strnchr()'s result can not be used as is. I was a bit
careless when writing my response.

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.