[tor-bugs] #6752 [Tor]: TestingTorNetwork doesn't lower the dir fetch retry schedules

Tor Bug Tracker & Wiki blackhole at torproject.org
Fri May 24 08:56:17 UTC 2013


#6752: TestingTorNetwork doesn't lower the dir fetch retry schedules
--------------------------------------+-------------------------------------
 Reporter:  arma                      |          Owner:                    
     Type:  enhancement               |         Status:  needs_review      
 Priority:  major                     |      Milestone:  Tor: 0.2.5.x-final
Component:  Tor                       |        Version:                    
 Keywords:  tor-client small-feature  |         Parent:  #7172             
   Points:                            |   Actualpoints:                    
--------------------------------------+-------------------------------------
Changes (by karsten):

  * status:  needs_revision => needs_review


Comment:

 Replying to [comment:43 nickm]:
 > Looks good; i want to get this in.

 Great!  Thanks for the code review!  See comments below.

 > Quick review:
 >  * The documentation for smartlist_ints_eq needs to say that the
 smartlists are lists of pointer to int.
 >  * There is a huge new amount of boilerplate copy-and-paste code in
 options_validate().  Please use macros or functions to avoid piles of
 duplicated code?
 >  * You added a new argument to options_validate but didn't document how
 it works.
 >  * Prefer smartlist_add_asprintf() to tor_asprintf(); smartlist_add().
 >  * find_dl_schedule_and_len should document its arguments and return
 types. (Yes, I know it didn't before, but it should.)
 >  * Shouldn't find_dl_schedule return a const smartlist_* ?

 Fixed all of these, I think.  Please see my updated branch.

 >  * It's weird that this function accesses Testing* options, but it's
 always used regardless of whether we're running in testing mode or not.  I
 don't much like having Testing* mean "a variable that you can only change
 when Testing* is on, but which is used in all cases."  Can we come up with
 some better way to do this?

 I agree that it feels strange to access Testing* options in code even in
 non-testing mode.  So, what we could do is remove the Testing* part of
 config option names and still require that they can only be changed when
 TestingTorNetwork is set; if you want me to do that, can you suggest how
 to tweak CHECK_DEFAULT to include the arg string in the message?

 However, from a user point of view, it's quite useful to realize quickly
 that one cannot change a config option unless running a testing network.
 The Testing prefix does a good job there, I think.

 I'm not sure what other ways there might be.  Hmm.

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


More information about the tor-bugs mailing list