[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #2149 [Tor Client]: new 'extra dormant' mode for people who never use their tor
#2149: new 'extra dormant' mode for people who never use their tor
---------------------------------+------------------------------------------
Reporter: arma | Owner:
Type: enhancement | Status: needs_revision
Priority: major | Milestone: Tor: 0.2.4.x-final
Component: Tor Client | Version:
Keywords: performance scaling | Parent:
Points: | Actualpoints:
---------------------------------+------------------------------------------
Changes (by nickm):
* status: needs_review => needs_revision
Comment:
Thanks for the code! Just pulled the branch -- The "Using wrong idle
checker" patch doesn't seem to be there yet?
Quick review:
* IDLE_TIME_STAGE isn't used.
* stopped_fetching_descriptors isn't documented.
* needs a changelog entry in the changes directory.
* or_options_t is for stuff parsed from torrc and the command line --
it isn't a catch-all for persistent state. That would be or_state_t.
Making this change will also let you back out lots of the
get_options_mutable() calls, which ought to be a red flag.
* I don't see the point of having DormantStage be global: why can't it
be a local variable in directory_too_idle_to_fetch_descriptors() ?
* I don't understand the logic of the changes in conn_read_callback and
do_main_loop. In any case, it feels like "directory_info_has_arrived()"
might be the wrong function to be calling here -- after all, you are
calling it *not* in response to directory info arriving.[*]
* The comment on 59c8df8b59bf04f25 seems wrong; conn_read_callback
isn't the function that gets called on "an incoming connection" -- it gets
called every time any socket is readable. If you want to have something
happen on incoming connections, that would belong in
connection_handle_listener_read, I think.
[*] Functions should be called as documented, and not because they have
some desirable side-effect. Let's say there is a function called
computer_is_on_fire() that floods the server room with carbon dioxide. It
is IMO wrong to call computer_is_on_fire() when the computer is not on
fire. If we sometimes want to flood the server room with CO2 when there
is no fire, then we should make an flood_server_room() function. That
makes our code more maintainable and less confusing.
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/2149#comment:14>
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