On Fri, 13 May 2022 15:31:20 +0900
Vincent Mailhol <[email protected]> wrote:
> On Fri. 13 May 2022 at 11:38, Vincent Mailhol
> <[email protected]> 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.
Max
On Sat. 14 mai 2022 at 03:59, Max Staudt <[email protected]> wrote:
> On Fri, 13 May 2022 15:31:20 +0900
> Vincent Mailhol <[email protected]> wrote:
> > On Fri. 13 May 2022 at 11:38, Vincent Mailhol
> > <[email protected]> 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.
On Sat, 14 May 2022 12:14:24 +0900
Vincent Mailhol <[email protected]> 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?
Max