diff --git a/changes/ticket30956_refactor b/changes/ticket30956_refactor
new file mode 100644
index 0000000000..81151c6cc9
--- /dev/null
+++ b/changes/ticket30956_refactor
@@ -0,0 +1,3 @@
+ o Code simplification and refactoring:
+ - Split extrainfo_dump_to_string() into smaller functions.
+ Closes ticket 30956.
diff --git a/scripts/maint/practracker/exceptions.txt b/scripts/maint/practracker/exceptions.txt
index 815d0ed097..4db452b897 100644
--- a/scripts/maint/practracker/exceptions.txt
+++ b/scripts/maint/practracker/exceptions.txt
@@ -225,13 +225,12 @@ problem function-size /src/feature/nodelist/routerlist.c:update_extrainfo_downlo
problem function-size /src/feature/relay/dns.c:dns_resolve_impl() 134
problem function-size /src/feature/relay/dns.c:configure_nameservers() 161
problem function-size /src/feature/relay/dns.c:evdns_callback() 109
-problem file-size /src/feature/relay/router.c 3407
+problem file-size /src/feature/relay/router.c 3510
problem include-count /src/feature/relay/router.c 56
problem function-size /src/feature/relay/router.c:init_keys() 252
problem function-size /src/feature/relay/router.c:get_my_declared_family() 114
problem function-size /src/feature/relay/router.c:router_build_fresh_unsigned_routerinfo() 136
problem function-size /src/feature/relay/router.c:router_dump_router_to_string() 371
-problem function-size /src/feature/relay/router.c:extrainfo_dump_to_string() 206
problem function-size /src/feature/relay/routerkeys.c:load_ed_keys() 294
problem function-size /src/feature/rend/rendcache.c:rend_cache_store_v2_desc_as_client() 193
problem function-size /src/feature/rend/rendclient.c:rend_client_send_introduction() 220
diff --git a/src/feature/relay/router.c b/src/feature/relay/router.c
index 25bb1835c2..6b33265294 100644
--- a/src/feature/relay/router.c
+++ b/src/feature/relay/router.c
@@ -3113,33 +3113,22 @@ load_stats_file(const char *filename, const char *end_line, time_t now,
return r;
}
-/** Write the contents of extrainfo, to * *s_out, signing them
- * with ident_key.
- *
- * If ExtraInfoStatistics is 1, also write aggregated statistics and related
- * configuration data before signing. Most statistics also have an option that
- * enables or disables that particular statistic.
- *
- * Return 0 on success, negative on failure. */
-int
-extrainfo_dump_to_string(char **s_out, extrainfo_t *extrainfo,
- crypto_pk_t *ident_key,
- const ed25519_keypair_t *signing_keypair)
+/** Add header strings to chunks, based on the extrainfo object extrainfo,
+ * and ed25519 keypair signing_keypair, if emit_ed_sigs is true.
+ * Helper for extrainfo_dump_to_string().
+ * Returns 0 on success, negative on failure. */
+static int
+extrainfo_dump_to_string_header_helper(
+ smartlist_t *chunks,
+ const extrainfo_t *extrainfo,
+ const ed25519_keypair_t *signing_keypair,
+ int emit_ed_sigs)
{
- const or_options_t *options = get_options();
char identity[HEX_DIGEST_LEN+1];
char published[ISO_TIME_LEN+1];
- char digest[DIGEST_LEN];
- int result;
- static int write_stats_to_extrainfo = 1;
- char sig[DIROBJ_MAX_SIG_LEN+1];
- char *s = NULL, *pre, *contents, *cp, *s_dup = NULL;
- time_t now = time(NULL);
- smartlist_t *chunks = smartlist_new();
- extrainfo_t *ei_tmp = NULL;
- const int emit_ed_sigs = signing_keypair &&
- extrainfo->cache_info.signing_key_cert;
char *ed_cert_line = NULL;
+ char *pre = NULL;
+ int rv = -1;
base16_encode(identity, sizeof(identity),
extrainfo->cache_info.identity_digest, DIGEST_LEN);
@@ -3175,6 +3164,29 @@ extrainfo_dump_to_string(char **s_out, extrainfo_t *extrainfo,
published);
smartlist_add(chunks, pre);
+ rv = 0;
+ goto done;
+
+ err:
+ rv = -1;
+
+ done:
+ tor_free(ed_cert_line);
+ return rv;
+}
+
+/** Add pluggable transport and statistics strings to chunks, skipping
+ * statistics if write_stats_to_extrainfo is false.
+ * Helper for extrainfo_dump_to_string().
+ * Can not fail. */
+static void
+extrainfo_dump_to_string_stats_helper(smartlist_t *chunks,
+ int write_stats_to_extrainfo)
+{
+ const or_options_t *options = get_options();
+ char *contents = NULL;
+ time_t now = time(NULL);
+
/* Add information about the pluggable transports we support, even if we
* are not publishing statistics. This information is needed by BridgeDB
* to distribute bridges. */
@@ -3241,21 +3253,113 @@ extrainfo_dump_to_string(char **s_out, extrainfo_t *extrainfo,
}
}
}
+}
+
+/** Add an ed25519 signature of chunks to chunks, using the ed25519 keypair
+ * signing_keypair.
+ * Helper for extrainfo_dump_to_string().
+ * Returns 0 on success, negative on failure. */
+static int
+extrainfo_dump_to_string_ed_sig_helper(
+ smartlist_t *chunks,
+ const ed25519_keypair_t *signing_keypair)
+{
+ char sha256_digest[DIGEST256_LEN];
+ ed25519_signature_t ed_sig;
+ char buf[ED25519_SIG_BASE64_LEN+1];
+ int rv = -1;
+
+ smartlist_add_strdup(chunks, "router-sig-ed25519 ");
+ crypto_digest_smartlist_prefix(sha256_digest, DIGEST256_LEN,
+ ED_DESC_SIGNATURE_PREFIX,
+ chunks, "", DIGEST_SHA256);
+ if (ed25519_sign(&ed_sig, (const uint8_t*)sha256_digest, DIGEST256_LEN,
+ signing_keypair) < 0)
+ goto err;
+ ed25519_signature_to_base64(buf, &ed_sig);
+
+ smartlist_add_asprintf(chunks, "%s\n", buf);
+
+ rv = 0;
+ goto done;
+
+ err:
+ rv = -1;
+
+ done:
+ return rv;
+}
+
+/** Add an RSA signature of extrainfo_string to chunks, using the RSA key
+ * ident_key.
+ * Helper for extrainfo_dump_to_string().
+ * Returns 0 on success, negative on failure. */
+static int
+extrainfo_dump_to_string_rsa_sig_helper(smartlist_t *chunks,
+ crypto_pk_t *ident_key,
+ const char *extrainfo_string)
+{
+ char sig[DIROBJ_MAX_SIG_LEN+1];
+ char digest[DIGEST_LEN];
+ int rv = -1;
+
+ memset(sig, 0, sizeof(sig));
+ if (router_get_extrainfo_hash(extrainfo_string, strlen(extrainfo_string),
+ digest) < 0 ||
+ router_append_dirobj_signature(sig, sizeof(sig), digest, DIGEST_LEN,
+ ident_key) < 0) {
+ log_warn(LD_BUG, "Could not append signature to extra-info "
+ "descriptor.");
+ goto err;
+ }
+ smartlist_add_strdup(chunks, sig);
+
+ rv = 0;
+ goto done;
+
+ err:
+ rv = -1;
+
+ done:
+ return rv;
+}
+
+/** Write the contents of extrainfo, to * *s_out, signing them
+ * with ident_key.
+ *
+ * If ExtraInfoStatistics is 1, also write aggregated statistics and related
+ * configuration data before signing. Most statistics also have an option that
+ * enables or disables that particular statistic.
+ *
+ * Always write pluggable transport lines.
+ *
+ * Return 0 on success, negative on failure. */
+int
+extrainfo_dump_to_string(char **s_out, extrainfo_t *extrainfo,
+ crypto_pk_t *ident_key,
+ const ed25519_keypair_t *signing_keypair)
+{
+ int result;
+ static int write_stats_to_extrainfo = 1;
+ char *s = NULL, *cp, *s_dup = NULL;
+ smartlist_t *chunks = smartlist_new();
+ extrainfo_t *ei_tmp = NULL;
+ const int emit_ed_sigs = signing_keypair &&
+ extrainfo->cache_info.signing_key_cert;
+ int rv = 0;
+
+ rv = extrainfo_dump_to_string_header_helper(chunks, extrainfo,
+ signing_keypair,
+ emit_ed_sigs);
+ if (rv < 0)
+ goto err;
+
+ extrainfo_dump_to_string_stats_helper(chunks, write_stats_to_extrainfo);
if (emit_ed_sigs) {
- char sha256_digest[DIGEST256_LEN];
- smartlist_add_strdup(chunks, "router-sig-ed25519 ");
- crypto_digest_smartlist_prefix(sha256_digest, DIGEST256_LEN,
- ED_DESC_SIGNATURE_PREFIX,
- chunks, "", DIGEST_SHA256);
- ed25519_signature_t ed_sig;
- char buf[ED25519_SIG_BASE64_LEN+1];
- if (ed25519_sign(&ed_sig, (const uint8_t*)sha256_digest, DIGEST256_LEN,
- signing_keypair) < 0)
+ rv = extrainfo_dump_to_string_ed_sig_helper(chunks, signing_keypair);
+ if (rv < 0)
goto err;
- ed25519_signature_to_base64(buf, &ed_sig);
-
- smartlist_add_asprintf(chunks, "%s\n", buf);
}
smartlist_add_strdup(chunks, "router-signature\n");
@@ -3285,15 +3389,10 @@ extrainfo_dump_to_string(char **s_out, extrainfo_t *extrainfo,
}
}
- memset(sig, 0, sizeof(sig));
- if (router_get_extrainfo_hash(s, strlen(s), digest) < 0 ||
- router_append_dirobj_signature(sig, sizeof(sig), digest, DIGEST_LEN,
- ident_key) < 0) {
- log_warn(LD_BUG, "Could not append signature to extra-info "
- "descriptor.");
+ rv = extrainfo_dump_to_string_rsa_sig_helper(chunks, ident_key, s);
+ if (rv < 0)
goto err;
- }
- smartlist_add_strdup(chunks, sig);
+
tor_free(s);
s = smartlist_join_strings(chunks, "", 0, NULL);
@@ -3329,7 +3428,6 @@ extrainfo_dump_to_string(char **s_out, extrainfo_t *extrainfo,
SMARTLIST_FOREACH(chunks, char *, chunk, tor_free(chunk));
smartlist_free(chunks);
tor_free(s_dup);
- tor_free(ed_cert_line);
extrainfo_free(ei_tmp);
return result;