[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Re: [tor-bugs] #12538 [Tor]: Make all relays automatically be dir caches



#12538: Make all relays automatically be dir caches
-------------------------------------------------+-------------------------
 Reporter:  cypherpunks                          |          Owner:
     Type:  task                                 |         Status:  closed
 Priority:  High                                 |      Milestone:  Tor:
Component:  Tor                                  |  0.2.8.x-final
 Severity:  Normal                               |        Version:  Tor:
 Keywords:  tor-guard, tor-relay, prop237,       |  unspecified
  026-triaged-1, sebastian-review,               |     Resolution:  fixed
  027-triaged-1-out, 028-triage, 028-triaged,    |  Actual Points:
  mike-can, pre028-patch, TorCoreTeam201512,     |         Points:
  201511-deferred                                |  medium/large
Parent ID:                                       |
  Sponsor:                                       |
-------------------------------------------------+-------------------------

Comment (by cypherpunks):

 Replying to [comment:98 nickm]:
 > Fixed the compilation problems and added a comment.
 Thanks

 >Not sure what you mean about the comments, or which comments you mean.
 I'll rephrase to add clarity.

 The have_enough_mem_for_dircache function modifies the total_mem parameter
 internally instead of using a separate variable throughout the function.
 Your added comment makes my comment on this obsolete because of the
 possibility that that piece of code will be removed. (A quick look at how
 the options_validate function assigns MaxMemInQueues suggests that it
 already does the necessary checks by calling
 compute_real_max_mem_in_queues making the checks in
 have_enough_mem_for_dircache redundant).

 My second point was about the get_total_system_memory function having a
 pointer to size_t parameter. Its internals use
 get_total_system_memory_impl which returns an uint64_t. The result is
 clamped to fit within the size_t range. My suggestion is to have
 get_total_system_memory have a pointer to uint64_t parameter so the result
 does not need to be clamped down. It would also remove the need for
 clamping in have_enough_mem_for_dircache.

 My last sub-point was about statements such as `if (0 == m) {` in
 get_total_system_memory and `if (-1 == (fd =
 tor_open_cloexec("/proc/meminfo",O_RDONLY,0)))` in
 get_total_system_memory_impl. These are called
 [https://en.wikipedia.org/wiki/Yoda_conditions Yoda conditions] and have
 some disadvantages as described in the
 [https://en.wikipedia.org/wiki/Yoda_conditions#Criticism Criticism
 section].

 If it's still not clear, I'm willing to submit patches for these points.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/12538#comment:99>
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