Hi Alexander,
On 6/24/09 11:19 AM, Alexander Nozdrin wrote:
> #At file:///mnt/raid/alik/MySQL/bzr/bug38247/azalea-bf-bug45584/ based on
> revid:alik@stripped
>
> 2804 Alexander Nozdrin 2009-06-24
> Fix for Bug#45584 (Host name cache does not work as a cache).
I guess the final patch will have detailed comments :-)
> modified:
> sql/hostname.cc
> sql/mysql_priv.h
> sql/sql_connect.cc
> === modified file 'sql/hostname.cc'
> --- a/sql/hostname.cc 2009-01-27 02:08:48 +0000
> +++ b/sql/hostname.cc 2009-06-24 14:19:40 +0000
> @@ -21,7 +21,7 @@
> Get hostname for an IP.
>
> Hostnames are checked with reverse name lookup and
> - checked that they doesn't resemble an ip.
> + checked that they doesn't resemble an IP.
> */
>
> #include "mysql_priv.h"
> @@ -40,12 +40,220 @@ extern "C" { // Because of SCO 3.2V4
> }
> #endif
>
> +#define HOST_ENTRY_KEY_SIZE NI_MAXHOST
> +
> +/*************************************************************************
> + On Windows IN6_IS_ADDR_LOOPBACK, IN6_IS_ADDR_V4MAPPED,
> + IN6_IS_ADDR_V4COMPAT are defined starting from Windows XP. The MySQL
> + server is supported starting from Windows 2000, where they are not
> + available. That's why we have to implement our own replacements.
> +**************************************************************************/
> +
> +static inline bool my_is_ip6_loopback(const struct in6_addr *ip6);
> +static inline bool my_is_ip6_v4_mapped(const struct in6_addr *ip6);
> +static inline bool my_is_ip6_v4_compat(const struct in6_addr *ip6);
Why is this necessary?
> +#if defined(__WIN__)&& defined(_WIN32_WINNT)&& _WIN32_WINNT<=
> 0x0500
> +
> +static inline bool my_is_ip6_loopback(const struct in6_addr *ip6)
> +{
> + return ip6->s6_words[0] == 0&&
> + ip6->s6_words[1] == 0&&
> + ip6->s6_words[2] == 0&&
> + ip6->s6_words[3] == 0&&
> + ip6->s6_words[4] == 0&&
> + ip6->s6_words[5] == 0&&
> + ip6->s6_words[6] == 0&&
> + ip6->s6_words[7] == 0x0100;
> +}
> +
> +static inline bool my_is_ip6_v4_mapped(const struct in6_addr *ip6)
> +{
> + return ip6->s6_words[0] == 0&&
> + ip6->s6_words[1] == 0&&
> + ip6->s6_words[2] == 0&&
> + ip6->s6_words[3] == 0&&
> + ip6->s6_words[4] == 0&&
> + ip6->s6_words[5] == 0xffff;
> +}
> +
> +static inline bool my_is_ip6_v4_compat(const struct in6_addr *ip6)
> +{
> + return ip6->s6_words[0] == 0&&
> + ip6->s6_words[1] == 0&&
> + ip6->s6_words[2] == 0&&
> + ip6->s6_words[3] == 0&&
> + ip6->s6_words[4] == 0&&
> + ip6->s6_words[5] == 0&&
> + (ip6->s6_words[6] != 0 ||
> + ip6->s6_addr[14] != 0 ||
> + ip6->s6_addr[15] != 0&& ip6->s6_addr[15] != 1);
> +}
> +
> +#else
> +
> +static inline bool my_is_ip6_loopback(const struct in6_addr *ip6)
> +{
> + return IN6_IS_ADDR_LOOPBACK(ip6);
> +}
> +
> +static inline bool my_is_ip6_v4_mapped(const struct in6_addr *ip6)
> +{
> + return IN6_IS_ADDR_V4MAPPED(ip6);
> +}
> +
> +static inline bool my_is_ip6_v4_compat(const struct in6_addr *ip6)
> +{
> + return IN6_IS_ADDR_V4COMPAT(ip6);
> +}
> +
> +#endif
Also, do we plan to support systems that don't have IPv6?
> +/************************************************************************/
> +
> +/*
> + When this code was written there were issues with winsock in pusbuild,
> + this constant is in this place for this reason.
> +*/
> +
> +#ifdef EAI_NODATA
> + const int MY_NONAME_ERR_CODE= EAI_NODATA;
> +#else
> + const int MY_NONAME_ERR_CODE= EAI_NONAME;
> +#endif
> +
> +/************************************************************************/
> +
> +/**
> + Get key value for IP address. Key value is a string representation of
> + normalized IP address.
> +
> + When IPv4 and IPv6 are both used in the network stack, or in the network
> + path between a client and a server, a client can have different apparent
> + IP addresses, based on the exact route taken.
> +
> + This function normalize the client IP representation, so it's suitable to
> + use as a key for searches.
> +
> + Transformations are implemented as follows:
> + - IPv4 a.b.c.d --> IPv6 mapped IPv4 ::ffff:a.b.c.d
> + - IPv6 compat IPv4 ::a.b.c.d --> IPv6 mapped IPv4 ::ffff:a.b.c.d
> + - IPv6 --> IPv6
If IPv6 is not supported on the system, we could use the IPv4.
> + @param [in] ip IP address
> + @param [out] ip_key Key for the given IP value
> +
> + @note According to RFC3493 the only specified member of the in6_addr
> + structure is s6_addr.
> +
> + @return Error status.
> + @retval FALSE Success
> + @retval TRUE Error
> +*/
> +
> +static bool hostname_cache_get_key(const struct sockaddr *ip, char *ip_key)
> +{
> + /* Prepare normalized IP address. */
> +
> + struct sockaddr_storage norm_ip;
> + struct in6_addr *norm_ip6=&((sockaddr_in6 *)&norm_ip)->sin6_addr;
> + uint32 *norm_ip6_int32= (uint32 *) norm_ip6->s6_addr;
> +
> + bzero(&norm_ip, sizeof (sockaddr_storage));
memset.
> + switch (ip->sa_family){
Space after ).
> + case AF_INET:
> + {
> + struct in_addr *ip4=&((struct sockaddr_in *) ip)->sin_addr;
> +
> + norm_ip6_int32[0]= 0;
> + norm_ip6_int32[1]= 0;
> + norm_ip6_int32[2]= htonl(0xffff);
> + norm_ip6_int32[3]= ip4->s_addr; /* in net byte order */
> +
> + DBUG_ASSERT(my_is_ip6_v4_mapped(norm_ip6));
> + break;
OK.
> + }
> +
> + case AF_INET6:
> + {
> + struct in6_addr *ip6=&((struct sockaddr_in6 *) ip)->sin6_addr;
> + uint32 *ip6_int32= (uint32 *) ip6->s6_addr;
> +
> + if (my_is_ip6_v4_compat(ip6))
> + {
> + norm_ip6_int32[0]= 0;
> + norm_ip6_int32[1]= 0;
> + norm_ip6_int32[2]= htonl(0xffff);
> + norm_ip6_int32[3]= ip6_int32[3]; /* in net byte order */
> + DBUG_ASSERT(my_is_ip6_v4_mapped(norm_ip6));
> + }
> + else
> + {
> + /* All in net byte order */
> + norm_ip6_int32[0]= ip6_int32[0];
> + norm_ip6_int32[1]= ip6_int32[1];
> + norm_ip6_int32[2]= ip6_int32[2];
> + norm_ip6_int32[3]= ip6_int32[3];
memcpy ?
> + }
> +
> + break;
> + }
> +
> + default:
> + DBUG_ASSERT(FALSE);
> + break;
> + }
We could ifdef out the above to support cases where IPv6 infrastructure
is not available.
> + /*
> + Zero all bytes of the key, because it's not just 0-terminated string.
> + All bytes are taken into account during hash search.
> + */
> +
> + bzero(ip_key, HOST_ENTRY_KEY_SIZE);
memset
> + /* Get numeric representation of the normalized IP address. */
> +
> + norm_ip.ss_family= AF_INET6;
> + return getnameinfo((struct sockaddr *)&norm_ip, sizeof (norm_ip),
> + ip_key, HOST_ENTRY_KEY_SIZE, NULL, 0,
> + NI_NUMERICHOST) != 0;
> +
> +}
OK.
> +/**
> + An entry in the hostname hash table cache.
> +
> + Host name cache does two things:
> + - caches host names to save DNS look ups;
> + - counts connect errors from IP.
> +
> + Host name can be NULL (that means DNS look up failed), but connect errors
> + still are counted.
> +*/
> +
> class host_entry :public hash_filo_element
> {
> public:
> - char ip[sizeof(struct sockaddr_storage)];
> - uint errors;
> - char *hostname;
> + /**
> + Client IP address. This is the key used with the hash table.
> +
> + The client IP address is always expressed in IPv6, even when the
> + network IPv6 stack is not present.
> +
> + This IP address is never used to connect to a socket.
> + */
> + char ip[HOST_ENTRY_KEY_SIZE];
> +
> + /**
> + Number of errors during handshake phase from the IP address.
> + */
> + uint connect_errors;
> +
> + /**
> + One of host names for the IP address. May be NULL.
> + */
> + char *hostname;
const char *hostname;
> };
>
> static hash_filo *hostname_cache;
> @@ -58,12 +266,14 @@ void hostname_cache_refresh()
> bool hostname_cache_init()
> {
> host_entry tmp;
> - uint offset= (uint) ((char*) (&tmp.ip) - (char*)&tmp);
As a side node, i'm not really sure if hash_filo_element can be
considered a POD type because of the constructor...
> - if (!(hostname_cache=new hash_filo(HOST_CACHE_SIZE, offset,
> - sizeof(struct sockaddr_storage),NULL,
> - (my_hash_free_key) free,
> - &my_charset_bin)))
> + uint key_offset= (uint) ((char*) (&tmp.ip) - (char*)&tmp);
> +
> + if (!(hostname_cache= new hash_filo(HOST_CACHE_SIZE,
> + key_offset, HOST_ENTRY_KEY_SIZE,
> + NULL, (my_hash_free_key) free,
> +&my_charset_bin)))
> return 1;
> +
> hostname_cache->clear();
>
> return 0;
> @@ -74,187 +284,378 @@ void hostname_cache_free()
> if (hostname_cache)
> {
> delete hostname_cache;
> - hostname_cache= 0;
> + hostname_cache= NULL;
> }
Actually, no need for the if. The operand of delete may be null.
> }
>
> -static void add_hostname(struct sockaddr_storage *in, const char *name)
> +static inline host_entry *hostname_cache_search(const char *ip)
> {
> - if (!(specialflag& SPECIAL_NO_HOST_CACHE))
> + return (host_entry *) hostname_cache->search((uchar *) ip, 0);
> +}
> +
> +static void add_hostname(const char *ip, const char *name)
name -> hostname
> +{
> + if (specialflag & SPECIAL_NO_HOST_CACHE)
> + return;
Check would be better on a higher level since we only invoke this
function twice..
> + pthread_mutex_lock(&hostname_cache->lock);
> + host_entry *entry;
> + if (!(entry= hostname_cache_search(ip)))
> {
> - pthread_mutex_lock(&hostname_cache->lock);
> - host_entry *entry;
> - if (!(entry=(host_entry*) hostname_cache->search((uchar*) in, 0)))
> + uint length=name ? (uint) strlen(name) : 0;
> +
> + if ((entry=(host_entry*) malloc(sizeof(host_entry)+length+1)))
> {
> - uint length=name ? (uint) strlen(name) : 0;
> + char *new_name;
> + memcpy_fixed(&entry->ip, ip, HOST_ENTRY_KEY_SIZE);
> + if (length)
> + {
> + memcpy(new_name= (char *) (entry+1), name, length+1);
>
> - if ((entry=(host_entry*) malloc(sizeof(host_entry)+length+1)))
> + DBUG_PRINT("info", ("Adding '%s' -> '%s' to the hostname cache...'",
> + (const char *) ip,
> + (const char *) new_name));
Do we need the casts? There are quite a few occurrences..
> + }
> + else
> {
> - char *new_name;
> - memcpy_fixed(&entry->ip, in, sizeof(struct sockaddr_storage));
> - if (length)
> - memcpy(new_name= (char *) (entry+1), name, length+1);
> - else
> - new_name=0;
> - entry->hostname=new_name;
> - entry->errors=0;
> - (void) hostname_cache->add(entry);
> + new_name=0;
0 -> NULL
> +
> + DBUG_PRINT("info", ("Adding '%s' -> NULL to the hostname cache...'",
> + (const char *) ip));
> }
> +
> + entry->hostname=new_name;
> + entry->connect_errors=0;
Space after =.
Both (this and above) ok since diff thinks it's moving.. heh.
> +
> + (void) hostname_cache->add(entry);
> }
> - pthread_mutex_unlock(&hostname_cache->lock);
> }
> + pthread_mutex_unlock(&hostname_cache->lock);
> }
>
> -inline void add_wrong_ip(struct sockaddr_storage *in)
> +static inline void add_unresolved_hostname(const char *ip)
> {
> - add_hostname(in, NullS);
> + add_hostname(ip, NULL);
IMHO, no need for this type of wrapper.
> }
>
> -void inc_host_errors(struct sockaddr_storage *in)
> +void inc_host_errors(struct sockaddr_storage *ip)
> {
> + char key[HOST_ENTRY_KEY_SIZE];
> +
> + if (hostname_cache_get_key((struct sockaddr *) ip, key))
> + {
> + DBUG_PRINT("error", ("Can not get key for IP address."));
> + return; /* Ignore error. */
> + }
> +
> pthread_mutex_lock(&hostname_cache->lock);
> - host_entry *entry;
>
> - if ((entry=(host_entry*) hostname_cache->search((uchar*)in, 0)))
> - entry->errors++;
> + host_entry *entry= hostname_cache_search(key);
> +
> + if (entry)
> + entry->connect_errors++;
>
> pthread_mutex_unlock(&hostname_cache->lock);
> }
>
>
> -void reset_host_errors(struct sockaddr_storage *in)
> +void reset_host_errors(struct sockaddr_storage *ip)
> {
> + char key[HOST_ENTRY_KEY_SIZE];
> +
> + if (hostname_cache_get_key((struct sockaddr *) ip, key))
> + {
> + DBUG_PRINT("error", ("Can not get key for IP address."));
> + return; /* Ignore error. */
> + }
> +
> pthread_mutex_lock(&hostname_cache->lock);
> - host_entry *entry;
>
> - if ((entry=(host_entry*) hostname_cache->search((uchar*)in, 0)))
> - entry->errors=0;
> + host_entry *entry= hostname_cache_search(key);
> +
> + if (entry)
> + entry->connect_errors=0;
>
> pthread_mutex_unlock(&hostname_cache->lock);
> }
>
>
> -char *ip_to_hostname(struct sockaddr_storage *in, int addrLen, uint *errors)
> +static inline bool is_ip_loopback(const struct sockaddr *ip)
> +{
> + switch (ip->sa_family){
> + case AF_INET:
> + {
> + /* Check for IPv4 127.0.0.1. */
> + struct in_addr *ip4_addr=&((struct sockaddr_in *) ip)->sin_addr;
> + return ip4_addr->s_addr== INADDR_LOOPBACK;
Space before ==.
> + }
> + case AF_INET6:
> + {
> + /* Check for IPv6 ::1. */
> + struct in6_addr *ip6_addr=&((struct sockaddr_in6 *) ip)->sin6_addr;
> + return my_is_ip6_loopback(ip6_addr);
> + }
> + default:
> + DBUG_ASSERT(FALSE);
> + return FALSE;
> + }
> +}
> +
> +static inline bool is_hostname_valid(const char *hostname)
> {
> - char *name;
> + if (!my_isdigit(&my_charset_latin1, hostname[0]))
> + return TRUE;
>
> - struct addrinfo hints,*res_lst,*t_res;
> - int gxi_error;
> - char hostname_buff[NI_MAXHOST];
> + const char *pos;
> +
> + for (pos= hostname + 1; my_isdigit(&my_charset_latin1, *pos); pos++)
> + ;
> +
> + return *pos == '.';
> +}
> +
> +/**
> + Resolve IP-address to host name.
> +
> + This function does the following things:
> + - resolves IP-address;
> + - employs Forward Confirmed Reverse DNS technique to validate IP-address;
> + - returns host name if IP-address is validated;
> + - set value to out-variable connect_errors -- this variable represents the
> + number of connection errors from the specified IP-address.
> +
> + NOTE: connect_errors are counted (are supported) only for the clients
> + where IP-address can be resolved and FCrDNS check is passed.
> +
> + @param [in] ip_storage
> + @param [out] hostname
> + @param [out] connect_errors
> +
> + @return Error status
> + @retval FALSE Success
> + @retval TRUE Error
> +
> + The function does not set/report MySQL server error in case of failure.
> + It's caller's responsibility to handle failures of this function
> + properly.
> +*/
> +
> +bool ip_to_hostname(struct sockaddr_storage *ip_storage,
> + char **hostname, uint *connect_errors)
> +{
> + const struct sockaddr *ip= (const sockaddr *) ip_storage;
> + char ip_string[HOST_ENTRY_KEY_SIZE];
> + int err_code;
>
> - host_entry *entry;
> DBUG_ENTER("ip_to_hostname");
> - *errors=0;
>
> - /* Historical comparison for 127.0.0.1 */
> - gxi_error= getnameinfo((struct sockaddr *)in, addrLen,
> - hostname_buff, NI_MAXHOST,
> - NULL, 0, NI_NUMERICHOST);
> - if (gxi_error)
> - {
> - DBUG_PRINT("error",("getnameinfo returned %d", gxi_error));
> - DBUG_RETURN(0);
> - }
> - DBUG_PRINT("info",("resolved: %s", hostname_buff));
> + /* Get hostname cache key for the IP address. */
>
> - /* The next three compares are to solve historical solutions with localhost */
> - if (!memcmp(hostname_buff, "127.0.0.1", sizeof("127.0.0.1")))
> + if (hostname_cache_get_key(ip, ip_string))
> {
> - DBUG_RETURN((char *)my_localhost);
> + DBUG_PRINT("error", ("Can not get key for IP address."));
> + return TRUE;
return TRUE -> DBUG_RETURN(TRUE)
Hum, we won't accept connections if getnameinfo can't return a IP
address... for example: http://tinyurl.com/l23dvx
Do we care? I'm inclined to not care, but we definitely print a warning.
> }
> - if (!memcmp(hostname_buff, "::ffff:127.0.0.1", sizeof("::ffff:127.0.0.1")))
> - {
> - DBUG_RETURN((char *)my_localhost);
> - }
> - if (!memcmp(hostname_buff, "::1", sizeof("::1")))
> +
> + DBUG_PRINT("info", ("IP address: '%s'.", (const char *) ip_string));
> +
> + /* Check if we have loopback address (127.0.0.1 or ::1). */
> +
> + if (is_ip_loopback(ip))
Shouldn't we check this first?
> {
> - DBUG_RETURN((char *)my_localhost);
> + DBUG_PRINT("info", ("IP address (%s) corresponds to loopback.",
> + (const char *) ip_string));
> +
> + *connect_errors= 0; /* Do not count connect errors from localhost. */
> + *hostname= (char *) my_localhost;
> +
> + DBUG_RETURN(FALSE);
> }
>
> - /* Check first if we have name in cache */
> + /* Check first if we have host name in the cache. */
> +
> if (!(specialflag& SPECIAL_NO_HOST_CACHE))
> {
> pthread_mutex_lock(&hostname_cache->lock);
> - if ((entry= (host_entry*)hostname_cache->search((uchar *)&in, 0)))
> +
> + host_entry *entry= hostname_cache_search(ip_string);
> +
> + if (entry)
> {
> + *connect_errors= entry->connect_errors;
> + *hostname= NULL;
> +
> if (entry->hostname)
> - name= my_strdup(entry->hostname, MYF(0));
> - else
> - name= NULL;
> + *hostname= my_strdup(entry->hostname, MYF(0));
> +
> + DBUG_PRINT("info",("IP (%s) has been found in the cache. "
> + "Hostname: '%s'; connect_errors: %d",
> + (const char *) ip_string,
> + (const char *) (*hostname? *hostname : "null"),
> + (int) *connect_errors));
>
> - DBUG_PRINT("info",("cached data %s", name ? name : "null" ));
> - *errors= entry->errors;
> pthread_mutex_unlock(&hostname_cache->lock);
> - DBUG_RETURN(name);
> +
> + DBUG_RETURN(FALSE);
> }
> +
> pthread_mutex_unlock(&hostname_cache->lock);
> }
>
> - if (!(name= my_strdup(hostname_buff, MYF(0))))
> + /* Resolve host name. */
> +
> + char hostname_buffer[NI_MAXHOST];
> +
> + DBUG_PRINT("info", ("Resolving '%s'...", (const char *) ip_string));
> +
> + err_code= getnameinfo(ip, sizeof (sockaddr_storage),
> + hostname_buffer, NI_MAXHOST, NULL, 0, 0);
> + if (err_code)
> {
> - DBUG_PRINT("error",("out of memory"));
> - DBUG_RETURN(0);
> + DBUG_PRINT("error",("getnameinfo returned %d", err_code));
We ought to print a warning for failures..
> + DBUG_RETURN(TRUE);
> }
>
> - /* Don't accept hostnames that starts with digits because they may be
> - false ip:s */
> - if (my_isdigit(&my_charset_latin1, name[0]))
> + DBUG_PRINT("info", ("IP '%s' resolved to '%s'.",
> + (const char *) ip_string,
> + (const char *) hostname_buffer));
> +
> + /*
> + Validate host name: we don't accept hostnames that starts with digits
> + because they may be false IPs.
> +
> + TODO: that check should be extended to support IPv6.
> + */
> +
> + if (!is_hostname_valid(hostname_buffer))
> {
> - char *pos;
> - for (pos= name+1 ; my_isdigit(&my_charset_latin1, *pos); pos++) ;
> - if (*pos == '.')
> - {
> - DBUG_PRINT("error",("mysqld doesn't accept hostnames that starts with a number
> followed by a '.'"));
> - goto add_wrong_ip_and_return;
> - }
> + DBUG_PRINT("error", ("mysqld doesn't accept hostnames that starts "
> + "with a number followed by a '.'"));
> +
> + add_unresolved_hostname(ip_string);
> + *hostname= NULL;
> + *connect_errors= 0; /* New IP added to the cache. */
> + DBUG_RETURN(TRUE);
> }
>
> + /* Get IP-addresses for the resolved host name (FCrDNS technique). */
> +
> + struct addrinfo hints;
> + struct addrinfo *addr_info_list;
> +
> bzero(&hints, sizeof (struct addrinfo));
> hints.ai_flags= AI_PASSIVE;
> - hints.ai_socktype= SOCK_STREAM;
> + hints.ai_socktype= SOCK_STREAM;
> hints.ai_family= AF_UNSPEC;
>
> - gxi_error= getaddrinfo(hostname_buff, NULL,&hints,&res_lst);
> - if (gxi_error)
> + DBUG_PRINT("info", ("Getting IP addresses for hostname '%s'...",
> + (const char *) hostname_buffer));
> +
> + err_code= getaddrinfo(hostname_buffer, NULL,&hints,&addr_info_list);
> +
> + if (err_code == MY_NONAME_ERR_CODE)
> {
> /*
> - Don't cache responses when the DSN server is down, as otherwise
> + Don't cache responses when the DNS server is down, as otherwise
> transient DNS failure may leave any number of clients (those
> that attempted to connect during the outage) unable to connect
> indefinitely.
> */
> - /*
> - When this code was written there were issues with winsock in pusbuild,
> - this define is in this place for this reason.
> - */
> - DBUG_PRINT("error",("getaddrinfo returned %d", gxi_error));
> -#ifdef EAI_NODATA
> - if (gxi_error == EAI_NODATA )
> -#else
> - if (gxi_error == EAI_NONAME )
> -#endif
> - add_wrong_ip(in);
>
> - my_free(name,MYF(0));
> - DBUG_RETURN(0);
> + add_unresolved_hostname(ip_string);
> + *hostname= NULL;
> + *connect_errors= 0; /* New IP added to the cache. */
> + DBUG_RETURN(FALSE);
> + }
> + else if (err_code)
> + {
> + DBUG_PRINT("error", ("getaddrinfo() failed with error code %d.", err_code));
> + DBUG_RETURN(TRUE);
> }
>
> - /* Check that 'getaddrinfo' returned the used ip */
> - for (t_res= res_lst; t_res; t_res=t_res->ai_next)
> + /* Check that getaddrinfo() returned the used IP (FCrDNS technique). */
> +
> + DBUG_PRINT("info", ("The following IP addresses found for '%s':",
> + (const char *) hostname_buffer));
> +
> + for (struct addrinfo *addr_info= addr_info_list;
> + addr_info; addr_info=addr_info->ai_next)
> +
> {
> - if (!memcmp(&(t_res->ai_addr), in,
> - sizeof(struct sockaddr_storage) ) )
> + struct sockaddr *resolved_ip= addr_info->ai_addr;
> + char resolved_ip_key[HOST_ENTRY_KEY_SIZE];
> +
> + if (hostname_cache_get_key(resolved_ip, resolved_ip_key))
Can't we still compare the addr structures?
> {
> - add_hostname(in, name);
> - freeaddrinfo(res_lst);
> - DBUG_RETURN(name);
> + DBUG_PRINT("error", ("Can not get key for IP address."));
> +
> + freeaddrinfo(addr_info_list);
> + DBUG_RETURN(TRUE);
> }
> +
> + DBUG_PRINT("info", (" - '%s'", (const char *) resolved_ip_key));
> +
> + if (strcmp(ip_string, resolved_ip_key) == 0)
> + {
> + /* Copy host name string to be stored in the cache. */
> +
> + *hostname= my_strdup(hostname_buffer, MYF(0));
> +
> + if (!*hostname)
> + {
> + DBUG_PRINT("error", ("Out of memory."));
> +
> + freeaddrinfo(addr_info_list);
> + DBUG_RETURN(TRUE);
> + }
> +
> + break;
> + }
> + }
> +
> + /* Log resolved IP-addresses if no match was found. */
> +
> + if (!*hostname)
> + {
> + sql_print_information("Hostname '%s' does not resolve to '%s'.",
> + (const char *) hostname_buffer,
> + (const char *) ip_string);
> + sql_print_information("Hostname '%s' has the following IP addresses:",
> + (const char *) hostname_buffer);
> +
> + for (struct addrinfo *addr_info= addr_info_list;
> + addr_info; addr_info=addr_info->ai_next)
IMHO, not needed. Admin can use dns tools and check.
> + {
> + struct sockaddr *resolved_ip= addr_info->ai_addr;
> + char resolved_ip_key[HOST_ENTRY_KEY_SIZE];
> +
> + if (hostname_cache_get_key(resolved_ip, resolved_ip_key))
> + strncpy(resolved_ip_key, "n/a", 3);
> +
> + sql_print_information(" - %s\n", (const char *) resolved_ip_key);
> + }
> + }
> +
> + /* Free the result of getaddrinfo(). */
> +
> + freeaddrinfo(addr_info_list);
> +
> + /* Add an entry for the IP to the cache. */
> +
> + if (*hostname)
> + {
> + add_hostname(ip_string, *hostname);
> + *connect_errors= 0;
> + }
> + else
> + {
> + DBUG_PRINT("error",("Couldn't verify hostname with getaddrinfo()."));
> +
> + add_unresolved_hostname(ip_string);
> + *hostname= NULL;
> + *connect_errors= 0;
> }
> - freeaddrinfo(res_lst);
> - DBUG_PRINT("error",("Couldn't verify hostname with getaddrinfo"));
>
> -add_wrong_ip_and_return:
> - my_free(name,MYF(0));
> - add_wrong_ip(in);
> - DBUG_RETURN(0);
> + DBUG_RETURN(FALSE);
> }
>
Looks good so far, thanks for working on this!
Regards,
