[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 nickm):

 Replying to [comment:99 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.

 (Thanks, and sorry for the delay! It's time off for me.)

 > 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.

 Hm. Seems like a good idea, but possibly another ticket.

 > 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].
 >

 Hm. I don't find the arguments on that page persuasive; I'm happy to let C
 programmers write `0 == m` or `NULL == x` or whatever, if that's what
 they're used to.

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