[tor-bugs] #8308 [Tor]: Use smartlist-of-strings rather than preallocated-charbuf for more directory stuff

[Tor Bug Tracker & Wiki]

#8308: Use smartlist-of-strings rather than preallocated-charbuf for more
directory stuff
--------------------------------+-------------------------------------------
 Reporter:  nickm               |          Owner:                    
     Type:  defect              |         Status:  needs_revision    
 Priority:  normal              |      Milestone:  Tor: 0.2.4.x-final
Component:  Tor                 |        Version:                    
 Keywords:  tor-auth tor-relay  |         Parent:                    
   Points:                      |   Actualpoints:                    
--------------------------------+-------------------------------------------
Changes (by asn):

  * status:  needs_review => needs_revision


Comment:

 Hm, `less_charbuf` changes dangerous parts of the code. More review should
 be needed. Good thing is that the code looks much much cleaner now.

 * `crypto_digest_smartlist()` has a `len_out` argument that gets passed to
 `crypto_digest_get_digest()` which in turn assumes that it is `<=
 DIGEST256_LEN`. Maybe this should be mentioned in the doc of
 `crypto_digest_smartlist()` too?

 * There is a DOCDOC lying around.

 * You use:
 {{{
 crypto_digest_smartlist(digest, digest_len, chunks, "", digest_alg)
 }}}
 do you want the NUL in all cases, or do you want:
 {{{
 crypto_digest_smartlist(digest, digest_len, chunks, NULL, digest_alg);
 }}}
 instead? (I'm not sure myself)

 * In some cases where the old code would error out, we now continue. Is
 this intended? Examples:

 {{{
 -    if (routerstatus_format_entry(outp, endp-outp, &vrs->status,
 +    if (routerstatus_format_entry(rs_buf, sizeof(rs_buf), &vrs->status,
                                    vrs->version, NS_V3_VOTE, vrs) < 0) {
 -      log_warn(LD_BUG, "Unable to print router status.");
 -      goto err;
 +      log_warn(LD_BUG, "Unable to print router status; skipping");
 +      continue;
 }}}

 {{{
 -      if (routerstatus_format_entry(outp, endp-outp, &rs, version, NS_V2,
 -                                    NULL)) {
 -        log_warn(LD_BUG, "Unable to print router status.");
 -        tor_free(version);
 -        goto done;
 +      {
 +        char *rsf = routerstatus_format_entry(&rs, version, NS_V2, NULL);
 +        if (rsf) {
 +          memcpy(outp, rsf, strlen(rsf)+1);
 +          outp += strlen(outp);
 +          tor_free(rsf);
 +        }
 }}}

 Marking as `needs_revision` for obvious things like the DOCDOC.

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