From eff9fbd17d5fb4b1c196c241da4513d51893f52e Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 7 Mar 2017 13:15:43 -0500 Subject: [PATCH] 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 2b1439569c..6fe8360902 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 diff_starting_line, 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; ilist = 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 3316ed6879..42cd3afa83 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 2afebfe2ab..12213aab64 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));