On Mon, 06 Feb 2017 15:05:32 -0800 evan.ga...@gmail.com (Evan Gates) wrote:
> Mattias Andrée <maand...@kth.se> wrote: > > > + } else if (escapes[*r & 255]) { > > + *w++ = escapes[*r++ & 255]; > > Why do you & 255 here? I think a cast to unsigned char > would accomplish what you are trying to do and be more > correct (as char can default to either signed or > unsigned). Although I may misunderstand what is going on > here. Yes. I used &255 because it's does clutter as much. > > > + q = q * 8 + (*r & 7); > > I think this is clearer: > > q = q * 8 + (*r - '0'); Go for it! > > > + *w++ = q > 255 ? 255 : q; > > Probably use MIN(q, 255) instead of the explicit ternary. Yeah, I forgot that we have MIN macro, so I didn't change that part. > > > + for (q = 0, m = 2; m && > > isxdigit(*r); m--, r++) > > + q = q * 16 + (*r & 15) > > + 9 * !!isalpha(*r); > > I think this is clearer, even though it adds a > conditional: > > q = q * 16 + isdigit(*r) ? *r - '0' : tolower(*r) - 'a' + > 10; I think if (isdigit(*r)) q = q * 16 + (*r - '0'); else q = q * 16 + (tolower(*r) - 'a') + 10; is clearer, or at least brackets around the ternary. > > Great work, much much simpler implementation. Once I > understand the &255 and we agree on the best solution for > these few lines I'll go ahead and push this.
pgp_aIrTP5Ed3.pgp
Description: OpenPGP digital signature