On Tue, Aug 16, 2022 at 08:58:37PM +0000, HushBugger wrote: > Thanks, I don't have a lot of practical C experience.
The reason for the cast is because <ctype.h> is a poorly designed library where the caller needs to ensure that the arg is representable as an `unsigned char` (i.e 0 .. UCHAR_MAX) or as `EOF` [0]. for (s = src, i = 0; *s; s++, i++) { - if (*s == '%' && (sscanf(s + 1, "%2hhx", &n) == 1)) { + if (*s == '%' && isxdigit((unsigned char)s[1]) && + isxdigit((unsigned char)s[2])) { + sscanf(s + 1, "%2hhx", &n); dest[i] = n; s += 2; At a glance, the `s += 2` looks incorrect to me because it's reading 3 bytes, not 2. But then I saw that the for loop increments `s` as well. IMO messing around with the loop counter in two places is not a good idea because it makes the code harder to reason about than it needs to be. I think the `s++` should be removed from the for loop and `s` should be incremented as needed inside the loop instead. - s += 2; + s += 3; } else { - dest[i] = *s; + dest[i] = *s++; But this is mostly style nit, so feel free to ignore. [0]: https://port70.net/~nsz/c/c11/n1570.html#7.4 - NRK