brbzull0 commented on a change in pull request #7874:
URL: https://github.com/apache/trafficserver/pull/7874#discussion_r639574437
##########
File path: iocore/hostdb/HostDB.cc
##########
@@ -2136,93 +2000,105 @@ HostDBFileContinuation::destroy()
// We can't allow more than one update to be
// proceeding at a time in any case so we might as well make these
// globals.
-int HostDBFileUpdateActive = 0;
+std::atomic<bool> HostDBFileUpdateActive{false};
+
+/* Container for temporarily holding data from the host file. For each FQDN
there is a vector of IPv4
+ * and IPv6 addresses. These are used to generate the HostDBRecord instances
that are stored persistently.
+ */
+using HostAddrMap = std::unordered_map<std::string_view,
std::tuple<std::vector<IpAddr>, std::vector<IpAddr>>>;
static void
-ParseHostLine(Ptr<RefCountedHostsFileMap> &map, char *l)
+ParseHostLine(TextView line, HostAddrMap &map)
{
- Tokenizer elts(" \t");
- int n_elts = elts.Initialize(l, SHARE_TOKS);
-
// Elements should be the address then a list of host names.
+ TextView addr_text = line.take_prefix_if(&isspace);
+ IpAddr addr;
+
// Don't use RecHttpLoadIp because the address *must* be literal.
- IpAddr ip;
- if (n_elts > 1 && 0 == ip.load(elts[0])) {
- for (int i = 1; i < n_elts; ++i) {
- ts::ConstBuffer name(elts[i], strlen(elts[i]));
- // If we don't have an entry already (host files only support single IPs
for a given name)
- if (map->hosts_file_map.find(name) == map->hosts_file_map.end()) {
- map->hosts_file_map[name] = ip;
- }
+ if (TS_SUCCESS != addr.load(addr_text)) {
+ return;
+ }
+
+ while (!line.ltrim_if(&isspace).empty()) {
+ TextView name = line.take_prefix_if(&isspace);
+ if (addr.isIp6()) {
+ std::get<1>(map[name]).push_back(addr);
+ } else if (addr.isIp4()) {
+ std::get<0>(map[name]).push_back(addr);
}
}
}
void
-ParseHostFile(const char *path, unsigned int
hostdb_hostfile_check_interval_parse)
+ParseHostFile(ts::file::path const &path, ts_seconds
hostdb_hostfile_check_interval_parse)
{
- Ptr<RefCountedHostsFileMap> parsed_hosts_file_ptr;
+ std::shared_ptr<HostFileMap> map;
+ std::string dbg;
// Test and set for update in progress.
- if (0 != ink_atomic_swap(&HostDBFileUpdateActive, 1)) {
+ bool flag = false;
+ if (!HostDBFileUpdateActive.compare_exchange_strong(flag, true)) {
Debug("hostdb", "Skipped load of host file because update already in
progress");
return;
}
- Debug("hostdb", "Loading host file '%s'", path);
-
- if (*path) {
- ats_scoped_fd fd(open(path, O_RDONLY));
- if (fd >= 0) {
- struct stat info;
- if (0 == fstat(fd, &info)) {
- // +1 in case no terminating newline
- int64_t size = info.st_size + 1;
-
- parsed_hosts_file_ptr = new RefCountedHostsFileMap;
- parsed_hosts_file_ptr->HostFileText = static_cast<char
*>(ats_malloc(size));
- if (parsed_hosts_file_ptr->HostFileText) {
- char *base = parsed_hosts_file_ptr->HostFileText;
- char *limit;
-
- size = read(fd, parsed_hosts_file_ptr->HostFileText, info.st_size);
- limit = parsed_hosts_file_ptr->HostFileText + size;
- *limit = 0;
-
- // We need to get a list of all name/addr pairs so that we can
- // group names for round robin records. Also note that the
- // pairs have pointer back in to the text storage for the file
- // so we need to keep that until we're done with @a pairs.
- while (base < limit) {
- char *spot = strchr(base, '\n');
-
- // terminate the line.
- if (nullptr == spot) {
- spot = limit; // no trailing EOL, grab remaining
- } else {
- *spot = 0;
- }
-
- while (base < spot && isspace(*base)) {
- ++base; // skip leading ws
- }
- if (*base != '#' && base < spot) { // non-empty non-comment line
- ParseHostLine(parsed_hosts_file_ptr, base);
- }
- base = spot + 1;
- }
-
- hostdb_hostfile_update_timestamp = hostdb_current_interval;
+ Debug("hostdb", "%s", ts::bwprint(dbg, R"(Loading host file "{}")",
path).c_str());
+
+ if (!path.empty()) {
+ std::error_code ec;
+ std::string content = ts::file::load(path, ec);
+ if (!ec) {
+ HostAddrMap addr_map;
+ TextView text{content};
+ while (text) {
+ auto line = text.take_prefix_at('\n').ltrim_if(&isspace);
+ if (line.empty() || '#' == *line) {
+ continue;
}
+ ParseHostLine(line, addr_map);
}
+ // @a map should be loaded with all of the data, create the records.
+ map.reset(new HostFileMap);
+ // Common loading function for creating a record from the address vector.
+ auto loader = [](TextView key, std::vector<IpAddr> const &v) ->
HostDBRecord::Handle {
+ HostDBRecord::Handle record{HostDBRecord::alloc(key, v.size())};
+ record->af_family = v.front().family(); // @a v is presumed family
homogenous
+ auto rr_info = record->rr_info();
+ auto spot = v.begin();
+ for (auto &item : rr_info) {
+ item.assign(*spot++);
+ }
+ return record;
+ };
+ // Walk the host file map and create the corresponding records.
+ for (auto const &[key, value] : addr_map) {
+ // Bit of subtlety to be able to search records with a view and not a
string - the key
+ // must point at stable memory for the name, which is available in the
record itself.
+ // Therefore the lookup for adding the record must be done using a
view based in the record.
+ // It doesn't matter if it's the IPv4 or IPv6 record that's used, both
are stable and equal
+ // to each other.
+ // IPv4
+ if (auto const &v = std::get<0>(value); v.size() > 0) {
Review comment:
would make sense to have a constant for IPv4 and IPv6 index??
```
namespace {
static constexpr int IPV4{0};
static constexpr int IPV6{1};
}
std::get<IPV4>(value)
std::get<IPV6>(value)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
