commit 9246a7ca58d14a975b2788772238c5a7799d54b6 Author: Nick Mathewson nickm@torproject.org Date: Wed Feb 20 00:55:34 2013 -0500
Refactor routerstatus_format_entry to avoid character-buffers --- src/or/dirserv.c | 108 ++++++++++++++++++------------------------------ src/or/dirserv.h | 2 +- src/or/dirvote.c | 21 ++++------ src/or/networkstatus.c | 4 +- 4 files changed, 51 insertions(+), 84 deletions(-)
diff --git a/src/or/dirserv.c b/src/or/dirserv.c index badacd6..472827a 100644 --- a/src/or/dirserv.c +++ b/src/or/dirserv.c @@ -2090,11 +2090,10 @@ version_from_platform(const char *platform) return NULL; }
-/** Helper: write the router-status information in <b>rs</b> into <b>buf</b>, - * which has at least <b>buf_len</b> free characters. Do NUL-termination. - * Use the same format as in network-status documents. If <b>version</b> is - * non-NULL, add a "v" line for the platform. Return 0 on success, -1 on - * failure. +/** Helper: write the router-status information in <b>rs</b> into a newly + * allocated character buffer. Use the same format as in network-status + * documents. If <b>version</b> is non-NULL, add a "v" line for the platform. + * Return 0 on success, -1 on failure. * * The format argument has one of the following values: * NS_V2 - Output an entry suitable for a V2 NS opinion document @@ -2105,25 +2104,25 @@ version_from_platform(const char *platform) * it contains additional information for the vote. * NS_CONTROL_PORT - Output a NS document for the control port */ -int -routerstatus_format_entry(char *buf, size_t buf_len, - const routerstatus_t *rs, const char *version, +char * +routerstatus_format_entry(const routerstatus_t *rs, const char *version, routerstatus_format_type_t format, const vote_routerstatus_t *vrs) { - int r; - char *cp; char *summary; + char *result = NULL;
char published[ISO_TIME_LEN+1]; char identity64[BASE64_DIGEST_LEN+1]; char digest64[BASE64_DIGEST_LEN+1]; + smartlist_t *chunks = NULL;
format_iso_time(published, rs->published_on); digest_to_base64(identity64, rs->identity_digest); digest_to_base64(digest64, rs->descriptor_digest);
- r = tor_snprintf(buf, buf_len, + chunks = smartlist_new(); + smartlist_add_asprintf(chunks, "r %s %s %s%s%s %s %d %d\n", rs->nickname, identity64, @@ -2133,11 +2132,6 @@ routerstatus_format_entry(char *buf, size_t buf_len, fmt_addr32(rs->addr), (int)rs->or_port, (int)rs->dir_port); - if (r<0) { - log_warn(LD_BUG, "Not enough space in buffer."); - return -1; - } - cp = buf + strlen(buf);
/* TODO: Maybe we want to pass in what we need to build the rest of * this here, instead of in the caller. Then we could use the @@ -2146,25 +2140,18 @@ routerstatus_format_entry(char *buf, size_t buf_len,
/* V3 microdesc consensuses don't have "a" lines. */ if (format == NS_V3_CONSENSUS_MICRODESC) - return 0; + goto done;
/* Possible "a" line. At most one for now. */ if (!tor_addr_is_null(&rs->ipv6_addr)) { - r = tor_snprintf(cp, buf_len - (cp-buf), - "a %s\n", - fmt_addrport(&rs->ipv6_addr, rs->ipv6_orport)); - if (r<0) { - log_warn(LD_BUG, "Not enough space in buffer."); - return -1; - } - cp += strlen(cp); + smartlist_add_asprintf(chunks, "a %s\n", + fmt_addrport(&rs->ipv6_addr, rs->ipv6_orport)); }
if (format == NS_V3_CONSENSUS) - return 0; + goto done;
- /* NOTE: Whenever this list expands, be sure to increase MAX_FLAG_LINE_LEN*/ - r = tor_snprintf(cp, buf_len - (cp-buf), + smartlist_add_asprintf(chunks, "s%s%s%s%s%s%s%s%s%s%s%s%s%s\n", /* These must stay in alphabetical order. */ rs->is_authority?" Authority":"", @@ -2180,20 +2167,11 @@ routerstatus_format_entry(char *buf, size_t buf_len, rs->is_unnamed?" Unnamed":"", rs->is_v2_dir?" V2Dir":"", rs->is_valid?" Valid":""); - if (r<0) { - log_warn(LD_BUG, "Not enough space in buffer."); - return -1; - } - cp += strlen(cp);
/* length of "opt v \n" */ #define V_LINE_OVERHEAD 7 if (version && strlen(version) < MAX_V_LINE_LEN - V_LINE_OVERHEAD) { - if (tor_snprintf(cp, buf_len - (cp-buf), "v %s\n", version)<0) { - log_warn(LD_BUG, "Unable to print router version."); - return -1; - } - cp += strlen(cp); + smartlist_add_asprintf(chunks, "v %s\n", version); }
if (format != NS_V2) { @@ -2213,7 +2191,7 @@ routerstatus_format_entry(char *buf, size_t buf_len, log_warn(LD_BUG, "Cannot get any descriptor for %s " "(wanted descriptor %s).", id, dd); - return -1; + goto err; }
/* This assert can fire for the control port, because @@ -2247,39 +2225,32 @@ routerstatus_format_entry(char *buf, size_t buf_len, tor_assert(desc); bw = router_get_advertised_bandwidth_capped(desc) / 1000; } - r = tor_snprintf(cp, buf_len - (cp-buf), - "w Bandwidth=%d\n", bw); + smartlist_add_asprintf(chunks, + "w Bandwidth=%d", bw);
- if (r<0) { - log_warn(LD_BUG, "Not enough space in buffer."); - return -1; - } - cp += strlen(cp); if (format == NS_V3_VOTE && vrs && vrs->has_measured_bw) { - *--cp = '\0'; /* Kill "\n" */ - r = tor_snprintf(cp, buf_len - (cp-buf), - " Measured=%d\n", vrs->measured_bw); - if (r<0) { - log_warn(LD_BUG, "Not enough space in buffer for weight line."); - return -1; - } - cp += strlen(cp); + smartlist_add_asprintf(chunks, + " Measured=%d", vrs->measured_bw); } + smartlist_add(chunks, tor_strdup("\n"));
if (desc) { summary = policy_summarize(desc->exit_policy, AF_INET); - r = tor_snprintf(cp, buf_len - (cp-buf), "p %s\n", summary); - if (r<0) { - log_warn(LD_BUG, "Not enough space in buffer."); - tor_free(summary); - return -1; - } - cp += strlen(cp); + smartlist_add_asprintf(chunks, "p %s\n", summary); tor_free(summary); } }
- return 0; + done: + result = smartlist_join_strings(chunks, "", 0, NULL); + + err: + if (chunks) { + SMARTLIST_FOREACH(chunks, char *, cp, tor_free(cp)); + smartlist_free(chunks); + } + + return result; }
/** Helper for sorting: compares two routerinfos first by address, and then by @@ -3054,14 +3025,15 @@ generate_v2_networkstatus_opinion(void) if (digestmap_get(omit_as_sybil, ri->cache_info.identity_digest)) clear_status_flags_on_sybil(&rs);
- 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); + } } tor_free(version); - outp += strlen(outp); } } SMARTLIST_FOREACH_END(ri);
diff --git a/src/or/dirserv.h b/src/or/dirserv.h index 0caf55f..bdd97d8 100644 --- a/src/or/dirserv.h +++ b/src/or/dirserv.h @@ -129,7 +129,7 @@ size_t dirserv_estimate_data_size(smartlist_t *fps, int is_serverdescs, int compressed); size_t dirserv_estimate_microdesc_size(const smartlist_t *fps, int compressed);
-int routerstatus_format_entry(char *buf, size_t buf_len, +char *routerstatus_format_entry( const routerstatus_t *rs, const char *platform, routerstatus_format_type_t format, const vote_routerstatus_t *vrs); diff --git a/src/or/dirvote.c b/src/or/dirvote.c index bd4e2f6..a787670 100644 --- a/src/or/dirvote.c +++ b/src/or/dirvote.c @@ -171,15 +171,12 @@ format_networkstatus_vote(crypto_pk_t *private_signing_key,
SMARTLIST_FOREACH_BEGIN(v3_ns->routerstatus_list, vote_routerstatus_t *, vrs) { -#define MAX_VOTE_ROUTERSTATUS_LEN 8192 - char rs_buf[MAX_VOTE_ROUTERSTATUS_LEN]; + char *rsf; vote_microdesc_hash_t *h; - 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; skipping"); - continue; - } - smartlist_add(chunks, tor_strdup(rs_buf)); + rsf = routerstatus_format_entry(&vrs->status, + vrs->version, NS_V3_VOTE, vrs); + if (rsf) + smartlist_add(chunks, rsf);
for (h = vrs->microdesc; h; h = h->next) { smartlist_add(chunks, tor_strdup(h->microdesc_hash_line)); @@ -1982,12 +1979,12 @@ networkstatus_compute_consensus(smartlist_t *votes, }
{ - char buf[4096]; + char *buf; /* Okay!! Now we can write the descriptor... */ /* First line goes into "buf". */ - routerstatus_format_entry(buf, sizeof(buf), &rs_out, NULL, - rs_format, NULL); - smartlist_add(chunks, tor_strdup(buf)); + buf = routerstatus_format_entry(&rs_out, NULL, rs_format, NULL); + if (buf) + smartlist_add(chunks, buf); } /* Now an m line, if applicable. */ if (flavor == FLAV_MICRODESC && diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c index c63c76f..cde1469 100644 --- a/src/or/networkstatus.c +++ b/src/or/networkstatus.c @@ -2127,9 +2127,7 @@ signed_descs_update_status_from_consensus_networkstatus(smartlist_t *descs) char * networkstatus_getinfo_helper_single(const routerstatus_t *rs) { - char buf[RS_ENTRY_LEN+1]; - routerstatus_format_entry(buf, sizeof(buf), rs, NULL, NS_CONTROL_PORT, NULL); - return tor_strdup(buf); + return routerstatus_format_entry(rs, NULL, NS_CONTROL_PORT, NULL); }
/** Alloc and return a string describing routerstatuses for the most