[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