List:Commits« Previous MessageNext Message »
From:Magne Mæhre Date:April 4 2011 1:26pm
Subject:Re: bzr commit into mysql-trunk branch (alexander.nozdrin:3330) WL#5787
View as plain text  
On 29/03/11 15:32, Alexander Nozdrin wrote:
> #At file:///home/alik/MySQL/bzr/00/wl5787/mysql-trunk-wl5787/ based on
> revid:alexander.nozdrin@stripped
> 
>  3330 Alexander Nozdrin	2011-03-29
>       Patch for WL#5787 (IPv6-capable INET_ATON and INET_NTOA functions).

Hi Alik.
This looks very good, so there are not much to comment on..

I have a few comments, anyway... And there are a couple of MS WIN
issues that needs to be adressed :)



> +static bool str_to_ipv4(const char *str, int str_length, in_addr *ipv4_address)
> +{
> +  if (str_length < 7)
> +  {
> +    DBUG_PRINT("error", ("str_to_ipv4(%s): "
> +                         "invalid IPv4 address: too short.", str));
> +    return false;
> +  }
> +
> +  if (str_length > 15)
> +  {
> +    DBUG_PRINT("error", ("str_to_ipv4(%s): "
> +                         "invalid IPv4 address: too long.", str));
> +    return false;
> +  }
> +
> +  unsigned char *ipv4_bytes = (unsigned char *) ipv4_address;
> +  const char *p = str;
> +  int byte_value = 0;
> +  int chars_in_group = 0;
> +  int dot_count = 0;

Code style: no space before '='  :)
  (there are a few more of these which I will not comment here)


> +  char c;
> +
> +  while (*p && (p - str < str_length))

Style issue:
I think I would suggest an additional pair of parenthesis here
   (p - str < str_length)   ->   ((p - str) < str_length)
just for clarity



> +  {
> +    c = *p++;
> +
> +    if (c >= '0' && c <= '9')


Would it be an idea to use   my_isdigit()  (or isdigit()) here ?


> +///////////////////////////////////////////////////////////////////////////
> +
> +/**
> +  Checks if the passed IPv6-address is an IPv4-compat IPv6-address.
> +
> +  @param arg The IPv6-address to check.
> +
> +  @return Check status.
> +  @retval false The passed IPv6-address is not an IPv4-compatible IPv6-address.
> +  @retval true  The passed IPv6-address is an IPv4-compatible IPv6-address.
> +*/
> +
> +bool Item_func_is_ipv4_compat::calc_value(const String *arg)
> +{
> +  if (arg->length() != 16 || arg->charset() != &my_charset_bin)
> +    return false;
> +
> +  return IN6_IS_ADDR_V4COMPAT(arg->ptr());


IN6_IS_ADDR_V4COMPAT takes a const struct in6_addr * as argument.
I believe we should use a reinterpret-cast here ?

(it will not compile on windows without a cast)



> +///////////////////////////////////////////////////////////////////////////
> +
> +/**
> +  Checks if the passed IPv6-address is an IPv4-mapped IPv6-address.
> +
> +  @param arg The IPv6-address to check.
> +
> +  @return Check status.
> +  @retval false The passed IPv6-address is not an IPv4-mapped IPv6-address.
> +  @retval true  The passed IPv6-address is an IPv4-mapped IPv6-address.
> +*/
> +
> +bool Item_func_is_ipv4_mapped::calc_value(const String *arg)
> +{
> +  if (arg->length() != 16 || arg->charset() != &my_charset_bin)
> +    return false;
> +
> +  return IN6_IS_ADDR_V4MAPPED(arg->ptr());

Same issue as above (IN6_IS_ADDR_V4COMPAT)

Thread
bzr commit into mysql-trunk branch (alexander.nozdrin:3330) WL#5787Alexander Nozdrin29 Mar
  • Re: bzr commit into mysql-trunk branch (alexander.nozdrin:3330) WL#5787Magne Mæhre4 Apr