[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #6752 [Tor]: TestingTorNetwork doesn't lower the dir fetch retry schedules
#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
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs