Re: [tor-bugs] #12205 [Tor]: Document magic numbers at entry_is_time_to_retry()

["Tor Bug Tracker & Wiki" <blackhole@xxxxxxxxxxxxxx>]

#12205: Document magic numbers at entry_is_time_to_retry()
-------------------------+--------------------------------
     Reporter:  asn      |      Owner:  rl1987
         Type:  defect   |     Status:  needs_review
     Priority:  trivial  |  Milestone:  Tor: 0.2.6.x-final
    Component:  Tor      |    Version:
   Resolution:           |   Keywords:  tor-guard
Actual Points:           |  Parent ID:
       Points:           |
-------------------------+--------------------------------

Comment (by nickm):

 I don't actually like this kind of stuff:
 {{{
  #define SECONDS_IN_AN_HOUR (60*60)
 }}}

 To me, it seems not much better than the classic:
 {{{
  #define FOUR (4)
 }}}

 The reason for using symbolic constants is to make intent clear.  When
 clarifying entry_is_time_to_retry, the question is not "what does 60*60
 mean" -- it's obviously an hour -- but rather "why are we looking at 60*60
 here?"

 Looking at the intent for this function, it is apparently to schedule
 attempts frequently at first, and then more slowly later on.  So maybe
 something like:
 {{{
     if (diff < ENTRY_RETRY_PERIOD_1)
       return now > (e->last_attempted + ENTRY_RETRY_INTERVAL_1)
     else if (diff < ENTRY_RETRY_PERIOD_2)
        ...

 }}}

 Or even make it table-driven, as in:
 {{{
     struct entry_retry_periods {
       time_t period_length;  time_t interval_during_period;
     } periods[] = {
      {    6*60*60,    60*60 }, /* For the first 6 hours, retry hourly. */
      { 3*24*60*60,  4*60*60 }, /* Then try every 4 hours until the 3-day
 mark. */
      { 7*24*60*60, 18*60*60 }, /* Then every 18 hours until the 7-day
 mark. */
      {  TIME_MAX,  36*60*60 }, /* Then every 36 hours thereafter. */
     }
     int i;
     for (i = 0; ; ++i) {
       if (diff <= periods[i].period_length)
         return now > (e->last_attempted +
 periods[i].interval_during_period) ;
     }
 }}}

 I dunno; what do you think?

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/12205#comment:8>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs