Mattias Andrée <[email protected]> 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.

> +                             q = q * 8 + (*r & 7);

I think this is clearer:

q = q * 8 + (*r - '0');

> +                     *w++ = q > 255 ? 255 : q;

Probably use MIN(q, 255) instead of the explicit ternary.

> +                     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;

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.

Reply via email to