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

Attachment: signature.asc
Description: PGP signature

-- 
## List details at https://lists.exim.org/mailman/listinfo/exim-dev Exim 
details at http://www.exim.org/ ##

Reply via email to