summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorNick Mathewson <nickm@torproject.org>2018-12-14 16:07:10 -0500
committerNick Mathewson <nickm@torproject.org>2018-12-14 16:07:10 -0500
commit3dd1f064a7d5708585f88beffaf3897ba6555208 (patch)
treeb4f9e9be7232be5a275c7f62c9d643f8fb45702e
parentf8dac5c900856494867996f60da848b0111aad35 (diff)
downloadtor-3dd1f064a7d5708585f88beffaf3897ba6555208.tar.gz
tor-3dd1f064a7d5708585f88beffaf3897ba6555208.zip
Rewrite the core of parse_short_policy() to be faster.
The old implementation did some funky out-of-order lexing, and tended to parse every port twice if the %d-%d pattern didn't match. Closes ticket 28853.
-rw-r--r--changes/ticket288533
-rw-r--r--src/core/or/policies.c80
2 files changed, 45 insertions, 38 deletions
diff --git a/changes/ticket28853 b/changes/ticket28853
new file mode 100644
index 0000000000..e76f6bd8c9
--- /dev/null
+++ b/changes/ticket28853
@@ -0,0 +1,3 @@
+ o Minor features (performance):
+ - Replace parse_short_policy() with a faster implementation, to improve
+ microdescriptor parsing time. Closes ticket 28853.
diff --git a/src/core/or/policies.c b/src/core/or/policies.c
index 123fc8566e..bffdb1fddd 100644
--- a/src/core/or/policies.c
+++ b/src/core/or/policies.c
@@ -2720,7 +2720,7 @@ parse_short_policy(const char *summary)
int is_accept;
int n_entries;
short_policy_entry_t entries[MAX_EXITPOLICY_SUMMARY_LEN]; /* overkill */
- const char *next;
+ char *next;
if (!strcmpstart(summary, "accept ")) {
is_accept = 1;
@@ -2735,57 +2735,56 @@ parse_short_policy(const char *summary)
n_entries = 0;
for ( ; *summary; summary = next) {
- const char *comma = strchr(summary, ',');
- unsigned low, high;
- char dummy;
- char ent_buf[32];
- size_t len;
-
- next = comma ? comma+1 : strchr(summary, '\0');
- len = comma ? (size_t)(comma - summary) : strlen(summary);
-
if (n_entries == MAX_EXITPOLICY_SUMMARY_LEN) {
log_fn(LOG_PROTOCOL_WARN, LD_DIR, "Impossibly long policy summary %s",
escaped(orig_summary));
return NULL;
}
- if (! TOR_ISDIGIT(*summary) || len > (sizeof(ent_buf)-1)) {
- /* unrecognized entry format. skip it. */
- continue;
- }
- if (len < 1) {
- /* empty; skip it. */
- /* XXX This happens to be unreachable, since if len==0, then *summary is
- * ',' or '\0', and the TOR_ISDIGIT test above would have failed. */
- continue;
+ unsigned low, high;
+ int ok;
+ low = (unsigned) tor_parse_ulong(summary, 10, 1, 65535, &ok, &next);
+ if (!ok) {
+ if (! TOR_ISDIGIT(*summary) || *summary == ',') {
+ /* Unrecognized format: skip it. */
+ goto skip_ent;
+ } else {
+ goto bad_ent;
+ }
}
- memcpy(ent_buf, summary, len);
- ent_buf[len] = '\0';
+ switch (*next) {
+ case ',':
+ ++next;
+ /* fall through */
+ case '\0':
+ high = low;
+ break;
+ case '-':
+ high = (unsigned) tor_parse_ulong(next+1, 10, low, 65535, &ok, &next);
+ if (!ok)
+ goto bad_ent;
+
+ if (*next == ',')
+ ++next;
+ else if (*next != '\0')
+ goto bad_ent;
- if (tor_sscanf(ent_buf, "%u-%u%c", &low, &high, &dummy) == 2) {
- if (low<1 || low>65535 || high<1 || high>65535 || low>high) {
- log_fn(LOG_PROTOCOL_WARN, LD_DIR,
- "Found bad entry in policy summary %s", escaped(orig_summary));
- return NULL;
- }
- } else if (tor_sscanf(ent_buf, "%u%c", &low, &dummy) == 1) {
- if (low<1 || low>65535) {
- log_fn(LOG_PROTOCOL_WARN, LD_DIR,
- "Found bad entry in policy summary %s", escaped(orig_summary));
- return NULL;
- }
- high = low;
- } else {
- log_fn(LOG_PROTOCOL_WARN, LD_DIR,"Found bad entry in policy summary %s",
- escaped(orig_summary));
- return NULL;
+ break;
+ default:
+ goto bad_ent;
}
entries[n_entries].min_port = low;
entries[n_entries].max_port = high;
n_entries++;
+
+ continue;
+ skip_ent:
+ next = strchr(next, ',');
+ if (!next)
+ break;
+ ++next;
}
if (n_entries == 0) {
@@ -2806,6 +2805,11 @@ parse_short_policy(const char *summary)
result->n_entries = n_entries;
memcpy(result->entries, entries, sizeof(short_policy_entry_t)*n_entries);
return result;
+
+ bad_ent:
+ log_fn(LOG_PROTOCOL_WARN, LD_DIR,"Found bad entry in policy summary %s",
+ escaped(orig_summary));
+ return NULL;
}
/** Write <b>policy</b> back out into a string. */