Messages in this thread | | | From | Vincent Mailhol <> | Date | Sat, 14 May 2022 12:14:24 +0900 | 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@enpas.org> wrote: > On Fri, 13 May 2022 15:31:20 +0900 > Vincent Mailhol <vincent.mailhol@gmail.com> wrote: > > On Fri. 13 May 2022 at 11:38, Vincent Mailhol > > <vincent.mailhol@gmail.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.
| |