#1138: If bridge authority is unreachable, client doesn't fallback to bridge
-------------------------+--------------------------------------------------
Reporter: arma | Owner: mwenge
Type: defect | Status: needs_review
Priority: major | Milestone: Tor: 0.2.2.x-final
Component: Tor Client | Version: 0.2.1.19
Resolution: None | Keywords: easy
Parent: |
-------------------------+--------------------------------------------------
Comment(by mwenge):
Sunday 29 August 2010] [16:22:17] Quit j27Z2 has left this server (Ping
timeout: 480 seconds).
[Sunday 29 August 2010] [16:33:10] Join j27Z2 has joined this channel
(~x@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx).
[Sunday 29 August 2010] [16:35:32] <Sebastian> mwenge: reviewing your
#1138 patch
[Sunday 29 August 2010] [16:35:48] <mwenge> thanks
[Sunday 29 August 2010] [16:36:20] <Sebastian> You're lacking
documentation for was_ei in connection_dir_retry_bridges()
[Sunday 29 August 2010] [16:36:42] <Sebastian> Also it seems better to not
have that parameter at all
[Sunday 29 August 2010] [16:36:57] <Sebastian> and rather still check the
assert at the caller
[Sunday 29 August 2010] [16:37:32] <Sebastian> Then you also don't need to
document it :)
[Sunday 29 August 2010] [16:38:44] <Sebastian> Calling the first argument
"failed" seems weird. Maybe just call it descs
[Sunday 29 August 2010] [16:39:22] Join alkovic has joined this channel
(~Undefined@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx).
[Sunday 29 August 2010] [16:40:33] <mwenge> ok
[Sunday 29 August 2010] [16:40:52] <Sebastian> (Please don't just update
the branch, but rather add a new patch to change these things. Maybe nickm
started reviewing it too)
[Sunday 29 August 2010] [16:40:58] <Sebastian> (We can still squash before
merge)
[Sunday 29 August 2010] [16:41:07] <mwenge> ah ok
[Sunday 29 August 2010] [16:41:56] <Sebastian> in
connection_dir_bridge_routerdesc_failed(), do we want to
tor_assert(conn->requested_resource) ?
[Sunday 29 August 2010] [16:42:48] <Sebastian> If we can't do that then we
have a bug
[Sunday 29 August 2010] [16:44:13] <Sebastian> What do you think?
[Sunday 29 August 2010] [16:46:52] <Sebastian> next is that you're saying
"conn->requested_resource + 3", please add a comment to say "skip over
fp/" or just do + strlen("fp/")
[Sunday 29 August 2010] [16:49:32] <Sebastian> and then finally I think
we're using names like connection_dir_bridge_routerdesc_failed() to ask
questions like "did this fail". Maybe we find a better name, maybe
something like connection_dir_bridge_routerdesc_handle_failure
[Sunday 29 August 2010] [16:49:34] <Sebastian> or such
[Sunday 29 August 2010] [16:50:00] <Sebastian> Ok please pick from this
list what you agree with :)
[Sunday 29 August 2010] [16:50:45] <mwenge> Sebastian: thanks for all
that, will work through them and respond on trac. i think i only need to
think about the was_ei one.
[Sunday 29 August 2010] [16:51:13] <Sebastian> Should I add everything to
trac?
[Sunday 29 August 2010] [16:51:26] <Sebastian> Happy to do that, though I
like discussing feedback before adding it.
[Sunday 29 August 2010] [16:52:03] <mwenge> no, it's ok i'll add it there.
i agree with everything.
I ended up disagreeing with the rename-functions suggestion. But the rest
of Sebastian's suggestions have been added to my bug1138 branch.
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/1138#comment:11>
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