Re: [tor-bugs] #17847 [Core Tor/Tor]: Unify router_pick_directory_server_impl and router_pick_trusteddirserver_impl

[Tor Bug Tracker & Wiki]

#17847: Unify router_pick_directory_server_impl and
router_pick_trusteddirserver_impl
-+-
 Reporter:  teor |  Owner:  ahf
 Type:  enhancement  | Status:  new
 Priority:  Medium   |  Milestone:  Tor:
 |  unspecified
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  easy, refactor, triaged- |  Actual Points:
  out-20170124   |
Parent ID:   | Points:  2
 Reviewer:  nickm|Sponsor:
-+-
Changes (by dgoulet):

 * milestone:  Tor: 0.3.2.x-final => Tor: unspecified


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #17847 [Core Tor/Tor]: Unify router_pick_directory_server_impl and router_pick_trusteddirserver_impl

[Tor Bug Tracker & Wiki]

#17847: Unify router_pick_directory_server_impl and
router_pick_trusteddirserver_impl
+
 Reporter:  teor|  Owner:  ahf
 Type:  enhancement | Status:  new
 Priority:  Medium  |  Milestone:  Tor: 0.3.0.x-final
Component:  Core Tor/Tor|Version:
 Severity:  Normal  | Resolution:
 Keywords:  easy, refactor  |  Actual Points:
Parent ID:  | Points:  2
 Reviewer:  nickm   |Sponsor:
+
Changes (by nickm):

 * status:  needs_review => new
 * keywords:  easy, refactor, review-group-15 => easy, refactor


Comment:

 Looks like a good start, and I'd be happy to take it as-is, but unless I'm
 missing something it doesn't yet actually unify  the two functions yet?
 They're still two separate functions with mostly duplicated logic.

 I've merged it, with a change to pass tor_addr_t by reference rather than
 on the stack. Calling this ticket "new" again.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #17847 [Core Tor/Tor]: Unify router_pick_directory_server_impl and router_pick_trusteddirserver_impl

[Tor Bug Tracker & Wiki]

#17847: Unify router_pick_directory_server_impl and
router_pick_trusteddirserver_impl
-+-
 Reporter:  teor |  Owner:  ahf
 Type:  enhancement  | Status:
 |  needs_review
 Priority:  Medium   |  Milestone:  Tor:
 |  0.3.0.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  easy, refactor, review-group-15  |  Actual Points:
Parent ID:   | Points:  2
 Reviewer:  nickm|Sponsor:
-+-
Changes (by nickm):

 * reviewer:   => nickm


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #17847 [Core Tor/Tor]: Unify router_pick_directory_server_impl and router_pick_trusteddirserver_impl

[Tor Bug Tracker & Wiki]

#17847: Unify router_pick_directory_server_impl and
router_pick_trusteddirserver_impl
+
 Reporter:  teor|  Owner:  ahf
 Type:  enhancement | Status:  needs_review
 Priority:  Medium  |  Milestone:  Tor: 0.3.0.x-final
Component:  Core Tor/Tor|Version:
 Severity:  Normal  | Resolution:
 Keywords:  easy, refactor  |  Actual Points:
Parent ID:  | Points:  2
 Reviewer:  |Sponsor:
+
Changes (by nickm):

 * status:  assigned => needs_review


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #17847 [Core Tor/Tor]: Unify router_pick_directory_server_impl and router_pick_trusteddirserver_impl

[Tor Bug Tracker & Wiki]

#17847: Unify router_pick_directory_server_impl and
router_pick_trusteddirserver_impl
+
 Reporter:  teor|  Owner:  ahf
 Type:  enhancement | Status:  assigned
 Priority:  Medium  |  Milestone:  Tor: 0.3.0.x-final
Component:  Core Tor/Tor|Version:
 Severity:  Normal  | Resolution:
 Keywords:  easy, refactor  |  Actual Points:
Parent ID:  | Points:  2
 Reviewer:  |Sponsor:
+
Changes (by nickm):

 * owner:   => ahf
 * status:  needs_review => assigned


Comment:

 Setting owner

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #17847 [Core Tor/Tor]: Unify router_pick_directory_server_impl and router_pick_trusteddirserver_impl

[Tor Bug Tracker & Wiki]

#17847: Unify router_pick_directory_server_impl and
router_pick_trusteddirserver_impl
+
 Reporter:  teor|  Owner:
 Type:  enhancement | Status:  needs_review
 Priority:  Medium  |  Milestone:  Tor: 0.3.0.x-final
Component:  Core Tor/Tor|Version:
 Severity:  Normal  | Resolution:
 Keywords:  easy, refactor  |  Actual Points:
Parent ID:  | Points:  2
 Reviewer:  |Sponsor:
+
Changes (by nickm):

 * status:  needs_revision => needs_review


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #17847 [Core Tor/Tor]: Unify router_pick_directory_server_impl and router_pick_trusteddirserver_impl

[Tor Bug Tracker & Wiki]

#17847: Unify router_pick_directory_server_impl and
router_pick_trusteddirserver_impl
+
 Reporter:  teor|  Owner:
 Type:  enhancement | Status:  needs_revision
 Priority:  Medium  |  Milestone:  Tor: 0.3.0.x-final
Component:  Core Tor/Tor|Version:
 Severity:  Normal  | Resolution:
 Keywords:  easy, refactor  |  Actual Points:
Parent ID:  | Points:  2
 Reviewer:  |Sponsor:
+

Comment (by ahf):

 Sorry for the delay, this month has been quite intense so far.

 I've made a revised version of the patch, which is now split into two
 patches, and can be found at:

 https://gitlab.com/ahf/tor/commits/ahf/bugs/17847_2

 I've removed all the whitespace changes, split the changes into two
 isolated commits, removed the `SKIP_IF_ALREADY_DIR_FETCHING` macro from
 the previous patch and instead generalised the function to support both of
 the variants that was needed before.

 I've removed the `SKIP_EXCLUDED` macro from the previous revision since I
 do not believe it actually adds any simplification to the code, since the
 body is just an increment and a `continue` statement.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #17847 [Core Tor/Tor]: Unify router_pick_directory_server_impl and router_pick_trusteddirserver_impl

[Tor Bug Tracker & Wiki]

#17847: Unify router_pick_directory_server_impl and
router_pick_trusteddirserver_impl
+
 Reporter:  teor|  Owner:
 Type:  enhancement | Status:  needs_revision
 Priority:  Medium  |  Milestone:  Tor: 0.3.0.x-final
Component:  Core Tor/Tor|Version:
 Severity:  Normal  | Resolution:
 Keywords:  easy, refactor  |  Actual Points:
Parent ID:  | Points:  2
 Reviewer:  |Sponsor:
+
Changes (by nickm):

 * points:   => 2


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #17847 [Core Tor/Tor]: Unify router_pick_directory_server_impl and router_pick_trusteddirserver_impl

[Tor Bug Tracker & Wiki]

#17847: Unify router_pick_directory_server_impl and
router_pick_trusteddirserver_impl
-+-
 Reporter:  teor |  Owner:
 Type:  enhancement  | Status:
 |  needs_revision
 Priority:  Medium   |  Milestone:  Tor:
 |  0.3.0.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  easy, refactor, review-group-12  |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+-
Changes (by teor):

 * status:  needs_review => needs_revision


Comment:

 Thanks for this patch!

 Many of the macros are missing arguments - it's confusing to have a macro
 use or modify a variable that isn't listed in its arguments.

 Almost all of SKIP_IF_ALREADY_DIR_FETCHING() could become a function,
 except for the body of the if statement, which is small enough to make it
 ok to duplicate.

 Some code just seems to move around or be reformatted - it might be better
 to put whitespace-only changes in their own commit.

 In general, these changes would be easier to review if each refactored
 macro or function was in its own commit.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #17847 [Core Tor/Tor]: Unify router_pick_directory_server_impl and router_pick_trusteddirserver_impl

[Tor Bug Tracker & Wiki]

#17847: Unify router_pick_directory_server_impl and
router_pick_trusteddirserver_impl
---+
 Reporter:  teor   |  Owner:
 Type:  enhancement| Status:  needs_review
 Priority:  Medium |  Milestone:  Tor: 0.3.0.x-final
Component:  Core Tor/Tor   |Version:
 Severity:  Normal | Resolution:
 Keywords:  easy refactor  |  Actual Points:
Parent ID: | Points:
 Reviewer: |Sponsor:
---+
Changes (by nickm):

 * status:  new => needs_review


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #17847 [Core Tor/Tor]: Unify router_pick_directory_server_impl and router_pick_trusteddirserver_impl

[Tor Bug Tracker & Wiki]

#17847: Unify router_pick_directory_server_impl and
router_pick_trusteddirserver_impl
---+
 Reporter:  teor   |  Owner:
 Type:  enhancement| Status:  new
 Priority:  Medium |  Milestone:  Tor: 0.3.0.x-final
Component:  Core Tor/Tor   |Version:
 Severity:  Normal | Resolution:
 Keywords:  easy refactor  |  Actual Points:
Parent ID: | Points:
 Reviewer: |Sponsor:
---+
Changes (by nickm):

 * milestone:  Tor: 0.2.??? => Tor: 0.3.0.x-final


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #17847 [Core Tor/Tor]: Unify router_pick_directory_server_impl and router_pick_trusteddirserver_impl

[Tor Bug Tracker & Wiki]

#17847: Unify router_pick_directory_server_impl and
router_pick_trusteddirserver_impl
---+--
 Reporter:  teor   |  Owner:
 Type:  enhancement| Status:  new
 Priority:  Medium |  Milestone:  Tor: 0.2.???
Component:  Core Tor/Tor   |Version:
 Severity:  Normal | Resolution:
 Keywords:  easy refactor  |  Actual Points:
Parent ID: | Points:
 Reviewer: |Sponsor:
---+--

Comment (by ahf):

 I have a partial fix, that could use a review and an iteration or two, in
 the `ahf/bugs/17847` branch on https://gitlab.com/ahf/tor.git

 Direct link to the branch:
 https://gitlab.com/ahf/tor/commits/ahf/bugs/17847

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs