Convert DNS search path to std::vector<SBuf> #1751
Conversation
rousskov
added
the
S-waiting-for-author
Co-authored-by: Alex Rousskov <rousskov@measurement-factory.com>
yadij
added
S-waiting-for-reviewer
rousskov
changed the title
| @@ -1745,7 +1755,7 @@ idnsALookup(const char *name, IDNSCB * callback, void *data) | |||
| if (q->do_searchpath && nd < ndots) { | |||
| q->domain = 0; | |||
| strcat(q->name, "."); | |||
| strcat(q->name, Dns::SearchPath()[q->domain].domain); | |||
| strcat(q->name, Dns::SearchPath()[q->domain].c_str()); | |||
There was a problem hiding this comment.
Here and elsewhere in this PR, please avoid SBuf::c_str() overheads by using strncat() instead of strcat(). I recommend encapsulating this duplicated code inside idns_query::useSearchPath() or similar method.
| Dns::SearchPath().push_back(name); | ||
|
|
||
| buf.chop(pos+Separator.length()); | ||
| auto pos = buf.find(Separator); |
There was a problem hiding this comment.
Typo?
| auto pos = buf.find(Separator); | |
| pos = buf.find(Separator); |
| name.toLower(); | ||
| debugs(78, DBG_IMPORTANT, "Adding domain " << name << " from hostname"); | ||
| Dns::SearchPath().push_back(name); |
There was a problem hiding this comment.
Please encapsulated this widely duplicated code into Dns::AddSearchPath(name, source) or similar function. We do not want to chaise places that forgot to call toLower() and/or debugs() when adding new search path components a few years from now.
| name.toLower(); | |
| debugs(78, DBG_IMPORTANT, "Adding domain " << name << " from hostname"); | |
| Dns::SearchPath().push_back(name); | |
| Dns::AddSearchPath(name, "from hostname"); |
The official code has idnsAddPathComponent(). This PR removes that function. We should replace it with a better one instead of inlining and duplicating the replacement code.
| @@ -419,6 +402,7 @@ idnsParseResolvConf(void) | |||
|
|
|||
| char buf[RESOLV_BUFSZ]; | |||
| const char *t = nullptr; | |||
| SBuf name; | |||
There was a problem hiding this comment.
Please declare this new variable for each use case that needs it, reducing future refactoring work to eliminate unwanted reuse of loop-global variables like t.
| @@ -404,6 +367,26 @@ idnsParseNameservers(void) | |||
| return result; | |||
| } | |||
|
|
|||
| static void | |||
| AddSearchPathFromHostname() | |||
There was a problem hiding this comment.
I recommend renaming this to MaybeAddSearchPathFromHostname() to avoid concerns that undocumented AddFoo() does not throw (or otherwise declare an error) when it does not actually add Foo. I do not insist on this change.
rousskov
added
S-waiting-for-author
squid-anubis
added
M-failed-other
No description provided.