Update Runner managing DNS components#2378
Conversation
yadij
added
the
S-waiting-for-reviewer
rousskov
left a comment
rousskov
left a comment
There was a problem hiding this comment.
This review is not comprehensive. I plan to come back to this PR after the backlog is dealt with.
I continue to beg you to stop posting new PRs until we find a way to deal with the large and growing backlog.
| void bootstrapConfig() override { | ||
| // XXX: replace globals | ||
| memset(RcodeMatrix, '\0', sizeof(RcodeMatrix)); | ||
| idns_lookup_hash = hash_create((HASHCMP *) strcmp, 103, hash_string); |
There was a problem hiding this comment.
These initialization steps are not necessary for configuration parsing and, hence, do not belong to bootstrapConfig() event. Most likely, idns_lookup_hash should be created like the other two hashes affected by this PR scope (fqdn_table and ip_table).
| idnsInitialize() | ||
| { | ||
| static int init = 0; | ||
| idnsParseEtcHosts(); |
There was a problem hiding this comment.
When working on improving this code, please do not add more configuration parsing steps into socket opening code. Squid should parse/validate configuration and then (if validation is successful) apply it (e.g., by closing and opening configuration-dependent sockets).
| ipcache_low = (long) (((float) Config.ipcache.size * | ||
| (float) Config.ipcache.low) / (float) 100); | ||
| n = hashPrime(ipcache_high / 4); | ||
| auto n = hashPrime(ipcache_high / 4); |
There was a problem hiding this comment.
If you are polishing how n is declared, please improve its const-correctness. The same applies to another function where PR changes how n is declared.
rousskov
removed
the
S-waiting-for-reviewer
2 participants
Dns::ConfigRr was already managing part of the DNS
internal component activity. Update it to fully control
the startup/reconfigure/shutdown actions.
This fixes some unnoticed bugs:
called before the DNS listening ports had been
updated and new nameservers loaded for use.
available for use when the component was disabled.
Matching /etc/resolv.conf, etc. load functions.