[tor-bugs] #14013 [Core Tor/Tor]: base16_decode() API is inconsistent and error-prone

[Tor Bug Tracker & Wiki]

#14013: base16_decode() API is inconsistent and error-prone
-----------------------------------+------------------------------------
 Reporter:  nickm                  |          Owner:  nikkolasg
     Type:  defect                 |         Status:  needs_revision
 Priority:  High                   |      Milestone:  Tor: 0.2.9.x-final
Component:  Core Tor/Tor           |        Version:
 Severity:  Normal                 |     Resolution:
 Keywords:  lorax, review-group-1  |  Actual Points:
Parent ID:                         |         Points:  small
 Reviewer:  dgoulet                |        Sponsor:  SponsorS-can
-----------------------------------+------------------------------------
Changes (by dgoulet):

 * status:  needs_review => needs_revision


Comment:

 First this is a good find. I've opened #19066 about it:

 {{{
 +    // XXX Should it not be always DIGEST256_LEN ? Running the tests with
 +    // the condition ` != DIGEST256_LEN` fails.
 }}}

 The patch looks good. There are some minor syntax issues that can easily
 be fixed such as:

 {{{
     if (base16_decode(digest, DIGEST_LEN, hexbuf, HEX_DIGEST_LEN)
             != DIGEST_LEN) {
 }}}

 It should be aligned like this to follow the code base:

 {{{
     if (base16_decode(digest, DIGEST_LEN,
                       hexbuf, HEX_DIGEST_LEN) != DIGEST_LEN) {
 }}}

 This one has an extra white space:

 {{{
            base16_decode(voter->identity_digest,
 sizeof(voter->identity_digest),
 -                        tok->args[1], HEX_DIGEST_LEN) < 0) {
 +                        tok->args[1], HEX_DIGEST_LEN)  != DIGEST_LEN) {
 }}}

 Align:

 {{{
 +    tt_int_op(sizeof(blinding_param), OP_EQ, DECODE(blinding_param,
 +                ED25519_BLINDING_PARAMS[i]));

 +    tt_int_op(sizeof(buf),OP_EQ,base16_decode(buf,sizeof(buf),\
 +                hex,strlen(hex)));\
 }}}

 Sorry about this, very wow such nitpicking but it makes the difference in
 the end for a more clean and maintainable code. Let me know if you can
 quickly fix those else I can go over them and credit you as the author.
 Thanks!

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/14013#comment:19>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online