[tor-bugs] #18873 [Core Tor/Tor]: Refactor circuit_predict_and_launch_new()

[Tor Bug Tracker & Wiki]

#18873: Refactor circuit_predict_and_launch_new()
---------------------------+------------------------------------
 Reporter:  asn            |          Owner:
     Type:  defect         |         Status:  merge_ready
 Priority:  Low            |      Milestone:  Tor: 0.3.0.x-final
Component:  Core Tor/Tor   |        Version:
 Severity:  Normal         |     Resolution:
 Keywords:  refactoring    |  Actual Points:
Parent ID:                 |         Points:
 Reviewer:  dgoulet, teor  |        Sponsor:
---------------------------+------------------------------------

Comment (by chelseakomlo):

 Hey Nick! Thanks for the review. I added these changes as new commits
 here: `git at github.com:chelseakomlo/tor_patches.git`, branch `circuituse`.

 > I'd request documentation on the extracted function in the unit tests,
 and on the new constants, but that isn't a showstopper, since nothing is
 getting _less_ documented in this patch.

 Great, I added documentation for these.

 > The indentation inside the "needs_circuits_for_build(num)" block needs
 to get cleaned up. Also, the spacing "needs_circuits_for_build" function
 itself violates the K&R C style we use elsewhere.

 I wasn't able to see the indentation issue here. I did fix the K&R style
 issue although i kept the inner brace (dgoulet style) but indented
 correctly.

 > For 0be9da8b6e016a00e234ad023513f44f7ba76843, instead of removing the
 check, I'd prefer to keep it, and add a BUG() wrapper around it.

 The BUG wrapper actually won't work here as this check isn't a bug, it
 just is redundant. I added this case back in the possibility that it is
 needed without being immediately obvious...

 One final thing I was thinking about is naming. In several places in the
 code, I use the term "hs servers" , but that probably isn't right. I've
 heard the term "hidden service routers" or just "hidden services." If you
 have consistent naming for these that you prefer, let me know and I can
 fix those up as well. For example, `needs_hs_server_circuits` and
 `SUFFICIENT_UPTIME_INTERNAL_HS_SERVERS`

 Thanks for all the help!

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