Re: Re: [PATCH 0/4] Refactor and expand string functions.

[Szabolcs Nagy <nsz@port70.net>]

* Rich Felker <dalias@aerifal.cx> [2013-02-05 09:05:35 -0500]:
> On Tue, Feb 05, 2013 at 12:19:10PM +0100, Szabolcs Nagy wrote:
> > * Nathan McSween <nwmcsween@gmail.com> [2013-02-04 20:25:53 -0800]:
> > > 	for (; (uintptr_t)cs % sizeof(size_t); cs++, n--) {
> > > 		if (!n) return NULL;
> > > 		if (*cs == c) return (void *)cs;
> > > 	}
> > > 
> > > 	for (w = (const size_t *)cs; !word_has_char(*w, c); w++, n--)
> > > 		if (!n) return NULL;
> > 
> > w++ but n-- does not seem right
> 
> You mean it should be n -= sizeof(size_t) or whatever?
> 

yes, or n -= sizeof *w if you want to save some chars :)

and the if (!n) return NULL; is wrong as well once you
go by words

> Honestly I would prefer the whole inline function be replaced with
> just a single-line macro that could be pasted where needed. IMO this
> makes the code more readable because you don't have to go looking at
> other functions.
> 

> word_has_char, but it's obvious what it does from the name. (Still, I
> prefer the original macro names, but that's just me; dunno what others
> think.)
> 

i agree

> > i dont see how word_has_char works, the name suggests that
> > it tests if w has c in it, but that's not what it does
> 
> That's what it's supposed to do.
> 

then the grouping is wrong

- return !!((w - WORD_LSB_ONE) & ((~w & WORD_MSB_ONE)^(WORD_LSB_ONE * c)));
+ return !!(((w - WORD_LSB_ONE) & ~w & WORD_MSB_ONE) ^ (WORD_LSB_ONE * c));

but yes the original macros were cleaner