[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20130205150541.GR6181@port70.net>
Date: Tue, 5 Feb 2013 16:05:41 +0100
From: Szabolcs Nagy <nsz@...t70.net>
To: musl@...ts.openwall.com
Subject: Re: Re: [PATCH 0/4] Refactor and expand string functions.
* Rich Felker <dalias@...ifal.cx> [2013-02-05 09:05:35 -0500]:
> On Tue, Feb 05, 2013 at 12:19:10PM +0100, Szabolcs Nagy wrote:
> > * Nathan McSween <nwmcsween@...il.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
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
