Re: [hackers] [PATCH v3][sbase] libutil/unescape.c: simplify and add \E

From: Evan Gates <evan.gates_AT_gmail.com>
Date: Mon, 06 Feb 2017 15:05:32 -0800

Mattias Andrée <maandree_AT_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.

> + 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.
Received on Tue Feb 07 2017 - 00:05:32 CET