[tor-commits] [tor/master] Refactor routerstatus_has_changed function

[teor at torproject.org]

commit 594e5ce635481a2aca60bbb31b656e3970ad511b
Author: Victor Nepveu <victor.nepveu at imt-atlantique.net>
Date:   Tue Nov 12 22:27:36 2019 +0100

    Refactor routerstatus_has_changed function
    
    - Check all fields that might change in a routerstatus
    - Document the refactoring
    
    Signed-off-by: Victor Nepveu <victor.nepveu at imt-atlantique.net>
---
 changes/ticket20218                     |  3 +++
 src/feature/nodelist/fmt_routerstatus.c |  2 ++
 src/feature/nodelist/networkstatus.c    | 22 +++++++++++++++++-----
 src/feature/nodelist/routerstatus_st.h  |  4 ++++
 4 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/changes/ticket20218 b/changes/ticket20218
new file mode 100644
index 000000000..d5fb2b2cf
--- /dev/null
+++ b/changes/ticket20218
@@ -0,0 +1,3 @@
+  o Minor bugfixes (controller):
+    - In routerstatus_has_changed(), check all the fields that are output over the control port.
+      Fixes bug 20218; bugfix on 0.1.1.11-alpha
diff --git a/src/feature/nodelist/fmt_routerstatus.c b/src/feature/nodelist/fmt_routerstatus.c
index 8dde0088d..dc44a6ef9 100644
--- a/src/feature/nodelist/fmt_routerstatus.c
+++ b/src/feature/nodelist/fmt_routerstatus.c
@@ -124,6 +124,8 @@ routerstatus_format_entry(const routerstatus_t *rs, const char *version,
     if (format != NS_CONTROL_PORT) {
       /* Blow up more or less nicely if we didn't get anything or not the
        * thing we expected.
+       * This should be kept in sync with the function
+       * routerstatus_has_changed and the struct routerstatus_t
        */
       if (!desc) {
         char id[HEX_DIGEST_LEN+1];
diff --git a/src/feature/nodelist/networkstatus.c b/src/feature/nodelist/networkstatus.c
index 786802047..df035ef1b 100644
--- a/src/feature/nodelist/networkstatus.c
+++ b/src/feature/nodelist/networkstatus.c
@@ -101,6 +101,7 @@
 #include "feature/nodelist/routerlist_st.h"
 #include "feature/dirauth/vote_microdesc_hash_st.h"
 #include "feature/nodelist/vote_routerstatus_st.h"
+#include "routerstatus_st.h"
 
 #ifdef HAVE_UNISTD_H
 #include <unistd.h>
@@ -1603,8 +1604,14 @@ networkstatus_consensus_has_ipv6(const or_options_t* options)
   }
 }
 
-/** Given two router status entries for the same router identity, return 1 if
- * if the contents have changed between them. Otherwise, return 0. */
+/** Given two router status entries for the same router identity, return 1
+ * if the contents have changed between them. Otherwise, return 0.
+ * It only checks for fields that are output by control port.
+ * This should be kept in sync with the struct routerstatus_t
+ * and the printing function routerstatus_format_entry in
+ * NS_CONTROL_PORT mode.
+ **/
+
 static int
 routerstatus_has_changed(const routerstatus_t *a, const routerstatus_t *b)
 {
@@ -1625,9 +1632,14 @@ routerstatus_has_changed(const routerstatus_t *a, const routerstatus_t *b)
          a->is_valid != b->is_valid ||
          a->is_possible_guard != b->is_possible_guard ||
          a->is_bad_exit != b->is_bad_exit ||
-         a->is_hs_dir != b->is_hs_dir;
-  // XXXX this function needs a huge refactoring; it has gotten out
-  // XXXX of sync with routerstatus_t, and it will do so again.
+         a->is_hs_dir != b->is_hs_dir ||
+         a->is_staledesc != b->is_staledesc ||
+         a->has_bandwidth != b->has_bandwidth ||
+         a->published_on != b->published_on ||
+         a->ipv6_orport != b->ipv6_orport ||
+         a->is_v2_dir != b->is_v2_dir ||
+         a->bandwidth_kb != b->bandwidth_kb ||
+         tor_addr_compare(&a->ipv6_addr, &b->ipv6_addr, CMP_EXACT);
 }
 
 /** Notify controllers of any router status entries that changed between
diff --git a/src/feature/nodelist/routerstatus_st.h b/src/feature/nodelist/routerstatus_st.h
index 270cb871a..eb6c3e5cf 100644
--- a/src/feature/nodelist/routerstatus_st.h
+++ b/src/feature/nodelist/routerstatus_st.h
@@ -17,6 +17,10 @@
 /** Contents of a single router entry in a network status object.
  */
 struct routerstatus_t {
+ /* This should be kept in sync with the function
+ * routerstatus_has_changed and the printing function
+ * routerstatus_format_entry in NS_CONTROL_PORT mode.
+ */
   time_t published_on; /**< When was this router published? */
   char nickname[MAX_NICKNAME_LEN+1]; /**< The nickname this router says it
                                       * has. */