Now this clang warning looks like it might actually be a bug :(
cc pdkim.c
pdkim.c:510:36: warning: expression result unused [-Wunused-value]
*c != isdigit(qp_p[1]) ? qp_p[1] - '0' : toupper(qp_p[1]) - 'A' + 10;
~~~~~~~ ^ ~~~
1 warning generated.
The upstream has
445 /* Check for two hex digits and decode them */
446 if (isxdigit(*qp_p) && isxdigit(qp_p[1])) {
447 /* Do hex conversion */
448 if (isdigit(*qp_p)) {*c = *qp_p - '0';}
449 else {*c = toupper(*qp_p) - 'A' + 10;}
450 *c <<= 4;
451 if (isdigit(qp_p[1])) {*c |= qp_p[1] - '0';}
452 else {*c |= toupper(qp_p[1]) - 'A' + 10;}
453 return qp_p + 2;
whereas Exim has
505 /* Check for two hex digits and decode them */
506 if (isxdigit(*qp_p) && isxdigit(qp_p[1]))
507 {
508 /* Do hex conversion */
509 *c = (isdigit(*qp_p) ? *qp_p - '0' : toupper(*qp_p) - 'A' + 10) << 4;
510 *c != isdigit(qp_p[1]) ? qp_p[1] - '0' : toupper(qp_p[1]) - 'A' + 10;
511 return qp_p + 2;
This doesn't look like a valid refactoring to me -- and clang is on to
something !
So the != needs to be a |=
(though if it was up to me I'd add brackets around the first subtraction for
clarity and write *qp_p as qp_p[0] to make the two expressions look the
same and hence be easier for people to read !
I assume, BTW, that the lack of ORing in the second value breaks something
somewhere, so there's test case(s) missing as well :-(
--
richard Richard Clayton
Those who would give up essential Liberty, to purchase a little temporary
Safety, deserve neither Liberty nor Safety. Benjamin Franklin 11 Nov 1755
