[tor-bugs] #16849 [Core Tor/Tor]: clear_status_flags_on_sybil might want to clear more flags

[Tor Bug Tracker & Wiki]

#16849: clear_status_flags_on_sybil might want to clear more flags
-------------------------------------------------+-------------------------
 Reporter:  teor                                 |          Owner:
                                                 |  ffmancera
     Type:  defect                               |         Status:
                                                 |  needs_revision
 Priority:  High                                 |      Milestone:  Tor:
                                                 |  0.3.4.x-final
Component:  Core Tor/Tor                         |        Version:
 Severity:  Normal                               |     Resolution:
 Keywords:  easy, SponsorS-deferred, technical-  |  Actual Points:
  debt, tor-dirauth, pending-disaster, review-   |
  group-32, review-group-34,                     |
  034-triage-20180328                            |
Parent ID:                                       |         Points:  small
 Reviewer:  nickm                                |        Sponsor:
-------------------------------------------------+-------------------------
Changes (by nickm):

 * status:  needs_review => needs_revision


Comment:

 Okay; sorry for all of the back-and-forth on this one.

 I've had another look at the code here, and talked it over with dgoulet,
 and we think that as it stands, this patch does not actually make the
 pattern here  less error-prone than it was before.  Previously, we had to
 be careful every time we added a new flag that would be cleared. With this
 patch, we must be careful every time we add a new field that would _not_
 be cleared.  The unit tests don't help us with the problem here, because
 they have no way to check for correct behavior of '''new fields''' in
 routerstatus_t.

 Most of the patterns that we thought of to try to improve the tests in
 this regard seem like they would cause other problems in the code (like
 forcing us to rewrite everything that uses the routerstatus_t flags).

 I'm putting this ticket into needs_revision, but it might need insight or
 a different approach entirely.  Even just some comments and might be an
 improvement.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/16849#comment:34>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online