If any hex strings contain letters above 'f' (or 'F'), we get an overflow. So for example,

unsigned char x;
p[0] = 'g';

x = (p[0] - 'a' + 10) << 4;

x would be 272, but it has a max of 256, so it ends up with a value of 16. Since x is unsigned char, that's not a problem in terms of C programming, but I think it's not the intended behaviour.

Given the existing code, I think the best thing to do in this case is to reject the string by having atohex return (-1).

Technically, the modified code still contains a bug -- some of those left-shifts need a cast in order to avoid undefined behaviour.

However, I think the git history will be much more clear if we make that change later (along with fixing other such problems). This particular PR changes program behaviour (if given a badly-formed checksum); the upcoming cast is merely addressing a theoretical problem (because the C standard doesn't require implementations to use two's complement for negative numbers ).