[tor-commits] [tor/master] Fix an abstraction violation.
nickm at torproject.org
nickm at torproject.org
Thu Mar 16 19:01:08 UTC 2017
commit eff9fbd17d5fb4b1c196c241da4513d51893f52e
Author: Nick Mathewson <nickm at torproject.org>
Date: Tue Mar 7 13:15:43 2017 -0500
Fix an abstraction violation.
Don't alias the insides of smartlist_t; that way lies madness.
---
src/or/consdiff.c | 25 +++++++++----------------
src/or/consdiff.h | 3 ++-
src/test/test_consdiff.c | 30 +++++++++++++++---------------
3 files changed, 26 insertions(+), 32 deletions(-)
diff --git a/src/or/consdiff.c b/src/or/consdiff.c
index 2b14395..6fe8360 100644
--- a/src/or/consdiff.c
+++ b/src/or/consdiff.c
@@ -654,18 +654,19 @@ gen_ed_diff(const smartlist_t *cons1, const smartlist_t *cons2)
return NULL;
}
-/** Apply the ed diff to the consensus and return a new consensus, also as a
- * line-based smartlist. Will return NULL if the ed diff is not properly
- * formatted.
+/** Apply the ed diff, starting at <b>diff_starting_line</b>, to the consensus
+ * and return a new consensus, also as a line-based smartlist. Will return
+ * NULL if the ed diff is not properly formatted.
*/
STATIC smartlist_t *
-apply_ed_diff(const smartlist_t *cons1, const smartlist_t *diff)
+apply_ed_diff(const smartlist_t *cons1, const smartlist_t *diff,
+ int diff_starting_line)
{
int diff_len = smartlist_len(diff);
int j = smartlist_len(cons1);
smartlist_t *cons2 = smartlist_new();
- for (int i=0; i<diff_len; ++i) {
+ for (int i=diff_starting_line; i<diff_len; ++i) {
const char *diff_line = smartlist_get(diff, i);
char *endptr1, *endptr2;
@@ -811,7 +812,7 @@ consdiff_gen_diff(const smartlist_t *cons1, const smartlist_t *cons2,
}
/* See that the script actually produces what we want. */
- smartlist_t *ed_cons2 = apply_ed_diff(cons1, ed_diff);
+ smartlist_t *ed_cons2 = apply_ed_diff(cons1, ed_diff, 0);
if (!ed_cons2) {
/* LCOV_EXCL_START -- impossible if diff generation is correct */
log_warn(LD_BUG|LD_CONSDIFF, "Refusing to generate consensus diff because "
@@ -978,16 +979,8 @@ consdiff_apply_diff(const smartlist_t *cons1,
}
/* Grab the ed diff and calculate the resulting consensus. */
- /* To avoid copying memory or iterating over all the elements, make a
- * read-only smartlist without the two header lines.
- */
- /* XXXX prop140 abstraction violation; never do this. */
- smartlist_t *ed_diff = tor_malloc(sizeof(smartlist_t));
- ed_diff->list = diff->list+2;
- ed_diff->num_used = diff->num_used-2;
- ed_diff->capacity = diff->capacity-2;
- cons2 = apply_ed_diff(cons1, ed_diff);
- tor_free(ed_diff);
+ /* Skip the first two lines. */
+ cons2 = apply_ed_diff(cons1, diff, 2);
/* ed diff could not be applied - reason already logged by apply_ed_diff. */
if (!cons2) {
diff --git a/src/or/consdiff.h b/src/or/consdiff.h
index 3316ed6..42cd3af 100644
--- a/src/or/consdiff.h
+++ b/src/or/consdiff.h
@@ -37,7 +37,8 @@ typedef struct smartlist_slice_t {
STATIC smartlist_t *gen_ed_diff(const smartlist_t *cons1,
const smartlist_t *cons2);
STATIC smartlist_t *apply_ed_diff(const smartlist_t *cons1,
- const smartlist_t *diff);
+ const smartlist_t *diff,
+ int start_line);
STATIC void calc_changes(smartlist_slice_t *slice1, smartlist_slice_t *slice2,
bitarray_t *changed1, bitarray_t *changed2);
STATIC smartlist_slice_t *smartlist_slice(const smartlist_t *list,
diff --git a/src/test/test_consdiff.c b/src/test/test_consdiff.c
index 2afebfe..12213aa 100644
--- a/src/test/test_consdiff.c
+++ b/src/test/test_consdiff.c
@@ -632,7 +632,7 @@ test_consdiff_apply_ed_diff(void *arg)
/* Command without range. */
smartlist_add(diff, (char*)"a");
- cons2 = apply_ed_diff(cons1, diff);
+ cons2 = apply_ed_diff(cons1, diff, 0);
tt_ptr_op(NULL, OP_EQ, cons2);
smartlist_clear(diff);
expect_single_log_msg_containing("an ed command was missing a line number");
@@ -640,7 +640,7 @@ test_consdiff_apply_ed_diff(void *arg)
/* Range without command. */
smartlist_add(diff, (char*)"1");
mock_clean_saved_logs();
- cons2 = apply_ed_diff(cons1, diff);
+ cons2 = apply_ed_diff(cons1, diff, 0);
tt_ptr_op(NULL, OP_EQ, cons2);
expect_single_log_msg_containing("a line with no ed command was found");
@@ -649,7 +649,7 @@ test_consdiff_apply_ed_diff(void *arg)
/* Range without end. */
smartlist_add(diff, (char*)"1,");
mock_clean_saved_logs();
- cons2 = apply_ed_diff(cons1, diff);
+ cons2 = apply_ed_diff(cons1, diff, 0);
tt_ptr_op(NULL, OP_EQ, cons2);
expect_single_log_msg_containing("an ed command was missing a range "
"end line number.");
@@ -659,7 +659,7 @@ test_consdiff_apply_ed_diff(void *arg)
/* Incoherent ranges. */
smartlist_add(diff, (char*)"1,1");
mock_clean_saved_logs();
- cons2 = apply_ed_diff(cons1, diff);
+ cons2 = apply_ed_diff(cons1, diff, 0);
tt_ptr_op(NULL, OP_EQ, cons2);
expect_single_log_msg_containing("an invalid range was found");
@@ -667,7 +667,7 @@ test_consdiff_apply_ed_diff(void *arg)
smartlist_add(diff, (char*)"3,2");
mock_clean_saved_logs();
- cons2 = apply_ed_diff(cons1, diff);
+ cons2 = apply_ed_diff(cons1, diff, 0);
tt_ptr_op(NULL, OP_EQ, cons2);
expect_single_log_msg_containing("an invalid range was found");
@@ -677,7 +677,7 @@ test_consdiff_apply_ed_diff(void *arg)
smartlist_add(diff, (char*)"1d");
smartlist_add(diff, (char*)"3d");
mock_clean_saved_logs();
- cons2 = apply_ed_diff(cons1, diff);
+ cons2 = apply_ed_diff(cons1, diff, 0);
tt_ptr_op(NULL, OP_EQ, cons2);
expect_single_log_msg_containing("its commands are not properly sorted");
@@ -686,7 +686,7 @@ test_consdiff_apply_ed_diff(void *arg)
/* Script contains unrecognised commands longer than one char. */
smartlist_add(diff, (char*)"1foo");
mock_clean_saved_logs();
- cons2 = apply_ed_diff(cons1, diff);
+ cons2 = apply_ed_diff(cons1, diff, 0);
tt_ptr_op(NULL, OP_EQ, cons2);
expect_single_log_msg_containing("an ed command longer than one char was "
"found");
@@ -696,7 +696,7 @@ test_consdiff_apply_ed_diff(void *arg)
/* Script contains unrecognised commands. */
smartlist_add(diff, (char*)"1e");
mock_clean_saved_logs();
- cons2 = apply_ed_diff(cons1, diff);
+ cons2 = apply_ed_diff(cons1, diff, 0);
tt_ptr_op(NULL, OP_EQ, cons2);
expect_single_log_msg_containing("an unrecognised ed command was found");
@@ -706,7 +706,7 @@ test_consdiff_apply_ed_diff(void *arg)
* isn't. */
smartlist_add(diff, (char*)"0a");
mock_clean_saved_logs();
- cons2 = apply_ed_diff(cons1, diff);
+ cons2 = apply_ed_diff(cons1, diff, 0);
tt_ptr_op(NULL, OP_EQ, cons2);
expect_single_log_msg_containing("it has an ed command that tries to "
"insert zero lines.");
@@ -714,7 +714,7 @@ test_consdiff_apply_ed_diff(void *arg)
/* Now it is followed by a ".", but it inserts zero lines. */
smartlist_add(diff, (char*)".");
mock_clean_saved_logs();
- cons2 = apply_ed_diff(cons1, diff);
+ cons2 = apply_ed_diff(cons1, diff, 0);
tt_ptr_op(NULL, OP_EQ, cons2);
expect_single_log_msg_containing("it has an ed command that tries to "
"insert zero lines.");
@@ -725,7 +725,7 @@ test_consdiff_apply_ed_diff(void *arg)
smartlist_add(diff, (char*)"0a");
smartlist_add(diff, (char*)"hello");
mock_clean_saved_logs();
- cons2 = apply_ed_diff(cons1, diff);
+ cons2 = apply_ed_diff(cons1, diff, 0);
tt_ptr_op(NULL, OP_EQ, cons2);
expect_single_log_msg_containing("lines to be inserted that don't end with "
"a \".\".");
@@ -734,7 +734,7 @@ test_consdiff_apply_ed_diff(void *arg)
/* Test appending text, 'a'. */
smartlist_split_string(diff, "3a:U:O:.:0a:V:.", ":", 0, 0);
- cons2 = apply_ed_diff(cons1, diff);
+ cons2 = apply_ed_diff(cons1, diff, 0);
tt_ptr_op(NULL, OP_NE, cons2);
tt_int_op(8, OP_EQ, smartlist_len(cons2));
tt_str_op("V", OP_EQ, smartlist_get(cons2, 0));
@@ -753,7 +753,7 @@ test_consdiff_apply_ed_diff(void *arg)
/* Test deleting text, 'd'. */
smartlist_split_string(diff, "4d:1,2d", ":", 0, 0);
- cons2 = apply_ed_diff(cons1, diff);
+ cons2 = apply_ed_diff(cons1, diff, 0);
tt_ptr_op(NULL, OP_NE, cons2);
tt_int_op(2, OP_EQ, smartlist_len(cons2));
tt_str_op("C", OP_EQ, smartlist_get(cons2, 0));
@@ -766,7 +766,7 @@ test_consdiff_apply_ed_diff(void *arg)
/* Test changing text, 'c'. */
smartlist_split_string(diff, "4c:T:X:.:1, 2c:M:.", ":", 0, 0);
- cons2 = apply_ed_diff(cons1, diff);
+ cons2 = apply_ed_diff(cons1, diff, 0);
tt_ptr_op(NULL, OP_NE, cons2);
tt_int_op(5, OP_EQ, smartlist_len(cons2));
tt_str_op("M", OP_EQ, smartlist_get(cons2, 0));
@@ -782,7 +782,7 @@ test_consdiff_apply_ed_diff(void *arg)
/* Test 'a', 'd' and 'c' together. */
smartlist_split_string(diff, "4c:T:X:.:2d:0a:M:.", ":", 0, 0);
- cons2 = apply_ed_diff(cons1, diff);
+ cons2 = apply_ed_diff(cons1, diff, 0);
tt_ptr_op(NULL, OP_NE, cons2);
tt_int_op(6, OP_EQ, smartlist_len(cons2));
tt_str_op("M", OP_EQ, smartlist_get(cons2, 0));
More information about the tor-commits
mailing list