Still reviewing, but I wish this had been a number of smaller patches. For
example, const'ing the MD5 stuff and adding ~PluginIdentity() could have been
independent patches ...
On Jul 24, 2014, at 7:20 PM, a...@apache.org wrote:
> Repository: trafficserver
> Updated Branches:
> refs/heads/master be428d6f4 -> 0c358a4b1
>
>
> TS-2863: Enable FQDN selection for shared sessions.
[snip]
> +bool
> +ServerSessionPool::match(HttpServerSession* ss, sockaddr const* addr,
> INK_MD5 const& hostname_hash, TSServerSessionSharingMatchType match_style)
> +{
> + return TS_SERVER_SESSION_SHARING_MATCH_NONE != match_style && // if no
> matching allowed, fail immediately.
> + // The hostname matches if we're not checking it or it (and the port!)
> is a match.
> + (TS_SERVER_SESSION_SHARING_MATCH_IP == match_style ||
> (ats_ip_port_cast(addr) == ats_ip_port_cast(ss->server_ip) &&
> ss->hostname_hash == hostname_hash)) &&
> + // The IP address matches if we're not checking it or it is a match.
> + (TS_SERVER_SESSION_SHARING_MATCH_HOST == match_style ||
> ats_ip_addr_port_eq(ss->server_ip, addr))
> + ;
> +}
The comments help, but IMHO this would still be easier to read in a more
declarative style:
switch (match_style) {
case TS_SERVER_SESSION_SHARING_MATCH_IP:
return ats_ip_port_cast(addr) == ats_ip_port_cast(ss->server_ip) &&
ss->hostname_hash == hostname_hash;
case TS_SERVER_SESSION_SHARING_MATCH_HOST:
return ats_ip_addr_port_eq(ss->server_ip, addr);
default:
ink_assert(match_style == TS_SERVER_SESSION_SHARING_MATCH_NONE);
return false;
}
