[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #9321 [Tor]: Load balance right when we have higher guard rotation periods
#9321: Load balance right when we have higher guard rotation periods
-------------------------+-------------------------------------------------
Reporter: arma | Owner:
Type: project | Status: needs_review
Priority: normal | Milestone: Tor: 0.2.6.x-final
Component: Tor | Version:
Resolution: | Keywords: needs-proposal, tor-auth, tor-
Actual Points: | client, 026-triaged-1
Points: | Parent ID: #11480
-------------------------+-------------------------------------------------
Comment (by teor):
Review of `git diff 390ec9154c58d6c7acaecd5e575adf575ab2f1fe^
asn/bug9321_draft` by reading through the modified code:
'''Impacts on other changes'''
If `dirserv_read_guardfraction_file` can handle reading an empty (zero-
length) file, and we merge #13111 in 0.2.6 as well, this code:
{{{
+ if (file_status(options->GuardfractionFile) != FN_FILE) {
+ REJECT("GuardfractionFile set but not a file? Failing");
+ }
+
+ dirserv_read_guardfraction_file(options->GuardfractionFile, NULL);
}}}
will need to be updated to:
{{{
+ if (file_status(options->GuardfractionFile) != FN_FILE &&
file_status(options->GuardfractionFile) != FN_EMPTY) {
+ REJECT("GuardfractionFile set but not a file? Failing");
+ }
+
+ dirserv_read_guardfraction_file(options->GuardfractionFile, NULL);
}}}
Or maybe we should just fail if the guardfraction file is truncated to
zero-length?
'''Minor Picky Things'''
Log Severities:
I'd downgrade `log_warn(LD_GENERAL, "%s: Guardfraction weight %f instead
of %f (%s)",` to a `log_debug`, given it may be called several thousand
times.
Integer ranges:
I think this code could cause an integer overflow on insane inputs when
`SIZEOF_LONG == SIZEOF_INT`:
{{{
version =
+ (unsigned int) tor_parse_long(line->value, 10, 0, UINT_MAX,
&num_ok, NU
LL);
}}}
`tor_parse_long` function prototype is:
{{{
long
tor_parse_long(const char *s, int base, long min, long max,
int *ok, char **next)
}}}
I suggest we use `tor_parse_long(line->value, 10, 0, INT_MAX, ...`.
Capitalisation:
* `UseGuardFraction` vs `GuardfractionFile` vs `Guardfraction:` in log
messages
* I suggest `GuardFractionFile` and `GuardFraction:` (all Fs
capitalised), but I don't care much either way about `Guard[Ff]raction:`
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/9321#comment:32>
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