RE: [PATCH V2 06/18] Tools: hv: Further refactor kvp_get_ip_address()

[KY Srinivasan]

> -Original Message-
> From: Ben Hutchings [mailto:b...@decadent.org.uk]
> Sent: Monday, August 13, 2012 9:46 PM
> To: KY Srinivasan
> Cc: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
> de...@linuxdriverproject.org; virtualizat...@lists.osdl.org; o...@aepfle.de;
> a...@canonical.com
> Subject: Re: [PATCH V2 06/18] Tools: hv: Further refactor kvp_get_ip_address()
> 
> On Mon, 2012-08-13 at 10:06 -0700, K. Y. Srinivasan wrote:
> > In preparation for making kvp_get_ip_address() more generic, factor out
> > the code for handling IP addresses.
> >
> > Signed-off-by: K. Y. Srinivasan 
> > Reviewed-by: Haiyang Zhang 
> > Reviewed-by: Olaf Hering 
> > Reviewed-by: Ben Hutchings 
> 
> I looked at your last patch set, so in a sense these have been reviewed
> by me.  But the 'Reviewed-by' line in a commit message means the
> reviewer thinks it's OK; the reviewer must say that, and I didn't.

My mistake, sorry about that.

Regards,

K. Y

Re: [PATCH V2 06/18] Tools: hv: Further refactor kvp_get_ip_address()

[Ben Hutchings]

On Mon, 2012-08-13 at 10:06 -0700, K. Y. Srinivasan wrote:
> In preparation for making kvp_get_ip_address() more generic, factor out
> the code for handling IP addresses.
> 
> Signed-off-by: K. Y. Srinivasan 
> Reviewed-by: Haiyang Zhang 
> Reviewed-by: Olaf Hering 
> Reviewed-by: Ben Hutchings 

I looked at your last patch set, so in a sense these have been reviewed
by me.  But the 'Reviewed-by' line in a commit message means the
reviewer thinks it's OK; the reviewer must say that, and I didn't.

Ben.

> ---
>  tools/hv/hv_kvp_daemon.c |   94 -
>  1 files changed, 42 insertions(+), 52 deletions(-)
> 
> diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
> index 3af37f0..3dc989f 100644
> --- a/tools/hv/hv_kvp_daemon.c
> +++ b/tools/hv/hv_kvp_daemon.c
> @@ -491,17 +491,50 @@ done:
>   return;
>  }
>  
> +static int kvp_process_ip_address(void *addrp,
> + int family, char *buffer,
> + int length,  int *offset)
> +{
> + struct sockaddr_in *addr;
> + struct sockaddr_in6 *addr6;
> + int addr_length;
> + char tmp[50];
> + const char *str;
> +
> + if (family == AF_INET) {
> + addr = (struct sockaddr_in *)addrp;
> + str = inet_ntop(family, >sin_addr, tmp, 50);
> + addr_length = INET_ADDRSTRLEN;
> + } else {
> + addr6 = (struct sockaddr_in6 *)addrp;
> + str = inet_ntop(family, >sin6_addr.s6_addr, tmp, 50);
> + addr_length = INET6_ADDRSTRLEN;
> + }
> +
> + if ((length - *offset) < addr_length + 1)
> + return 1;
> + if (str == NULL) {
> + strcpy(buffer, "inet_ntop failed\n");
> + return 1;
> + }
> + if (*offset == 0)
> + strcpy(buffer, tmp);
> + else
> + strcat(buffer, tmp);
> + strcat(buffer, ";");
> +
> + *offset += strlen(str) + 1;
> + return 0;
> +}
> +
>  static int
>  kvp_get_ip_address(int family, char *if_name, int op,
>void  *out_buffer, int length)
>  {
>   struct ifaddrs *ifap;
>   struct ifaddrs *curp;
> - int ipv4_len = strlen("255.255.255.255") + 1;
> - int ipv6_len = strlen(":::::::")+1;
>   int offset = 0;
>   const char *str;
> - char tmp[50];
>   int error = 0;
>   char *buffer;
>   struct hv_kvp_ipaddr_value *ip_buffer;
> @@ -556,55 +589,12 @@ kvp_get_ip_address(int family, char *if_name, int op,
>   continue;
>   }
>  
> - if ((curp->ifa_addr->sa_family == AF_INET) &&
> - ((family == AF_INET) || (family == 0))) {
> - struct sockaddr_in *addr =
> - (struct sockaddr_in *) curp->ifa_addr;
> -
> - str = inet_ntop(AF_INET, >sin_addr, tmp, 50);
> - if (str == NULL) {
> - strcpy(buffer, "inet_ntop failed\n");
> - error = 1;
> - goto getaddr_done;
> - }
> - if (offset == 0)
> - strcpy(buffer, tmp);
> - else
> - strcat(buffer, tmp);
> - strcat(buffer, ";");
> -
> - offset += strlen(str) + 1;
> - if ((length - offset) < (ipv4_len + 1))
> - goto getaddr_done;
> -
> - } else if ((family == AF_INET6) || (family == 0)) {
> -
> - /*
> -  * We only support AF_INET and AF_INET6
> -  * and the list of addresses is separated by a ";".
> -  */
> - struct sockaddr_in6 *addr =
> - (struct sockaddr_in6 *) curp->ifa_addr;
> -
> - str = inet_ntop(AF_INET6,
> - >sin6_addr.s6_addr,
> - tmp, 50);
> - if (str == NULL) {
> - strcpy(buffer, "inet_ntop failed\n");
> - error = 1;
> - goto getaddr_done;
> - }
> - if (offset == 0)
> - strcpy(buffer, tmp);
> - else
> - strcat(buffer, tmp);
> - strcat(buffer, ";");
> - offset += strlen(str) + 1;
> - if ((length - offset) < (ipv6_len + 1))
> - goto getaddr_done;
> -
> - }
> -
> + error = kvp_process_ip_address(curp->ifa_addr,
> + curp->ifa_addr->sa_family,
> + buffer,
> + length, );
> 

[PATCH V2 06/18] Tools: hv: Further refactor kvp_get_ip_address()

[K. Y. Srinivasan]

In preparation for making kvp_get_ip_address() more generic, factor out
the code for handling IP addresses.

Signed-off-by: K. Y. Srinivasan 
Reviewed-by: Haiyang Zhang 
Reviewed-by: Olaf Hering 
Reviewed-by: Ben Hutchings 
---
 tools/hv/hv_kvp_daemon.c |   94 -
 1 files changed, 42 insertions(+), 52 deletions(-)

diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
index 3af37f0..3dc989f 100644
--- a/tools/hv/hv_kvp_daemon.c
+++ b/tools/hv/hv_kvp_daemon.c
@@ -491,17 +491,50 @@ done:
return;
 }
 
+static int kvp_process_ip_address(void *addrp,
+   int family, char *buffer,
+   int length,  int *offset)
+{
+   struct sockaddr_in *addr;
+   struct sockaddr_in6 *addr6;
+   int addr_length;
+   char tmp[50];
+   const char *str;
+
+   if (family == AF_INET) {
+   addr = (struct sockaddr_in *)addrp;
+   str = inet_ntop(family, >sin_addr, tmp, 50);
+   addr_length = INET_ADDRSTRLEN;
+   } else {
+   addr6 = (struct sockaddr_in6 *)addrp;
+   str = inet_ntop(family, >sin6_addr.s6_addr, tmp, 50);
+   addr_length = INET6_ADDRSTRLEN;
+   }
+
+   if ((length - *offset) < addr_length + 1)
+   return 1;
+   if (str == NULL) {
+   strcpy(buffer, "inet_ntop failed\n");
+   return 1;
+   }
+   if (*offset == 0)
+   strcpy(buffer, tmp);
+   else
+   strcat(buffer, tmp);
+   strcat(buffer, ";");
+
+   *offset += strlen(str) + 1;
+   return 0;
+}
+
 static int
 kvp_get_ip_address(int family, char *if_name, int op,
 void  *out_buffer, int length)
 {
struct ifaddrs *ifap;
struct ifaddrs *curp;
-   int ipv4_len = strlen("255.255.255.255") + 1;
-   int ipv6_len = strlen(":::::::")+1;
int offset = 0;
const char *str;
-   char tmp[50];
int error = 0;
char *buffer;
struct hv_kvp_ipaddr_value *ip_buffer;
@@ -556,55 +589,12 @@ kvp_get_ip_address(int family, char *if_name, int op,
continue;
}
 
-   if ((curp->ifa_addr->sa_family == AF_INET) &&
-   ((family == AF_INET) || (family == 0))) {
-   struct sockaddr_in *addr =
-   (struct sockaddr_in *) curp->ifa_addr;
-
-   str = inet_ntop(AF_INET, >sin_addr, tmp, 50);
-   if (str == NULL) {
-   strcpy(buffer, "inet_ntop failed\n");
-   error = 1;
-   goto getaddr_done;
-   }
-   if (offset == 0)
-   strcpy(buffer, tmp);
-   else
-   strcat(buffer, tmp);
-   strcat(buffer, ";");
-
-   offset += strlen(str) + 1;
-   if ((length - offset) < (ipv4_len + 1))
-   goto getaddr_done;
-
-   } else if ((family == AF_INET6) || (family == 0)) {
-
-   /*
-* We only support AF_INET and AF_INET6
-* and the list of addresses is separated by a ";".
-*/
-   struct sockaddr_in6 *addr =
-   (struct sockaddr_in6 *) curp->ifa_addr;
-
-   str = inet_ntop(AF_INET6,
-   >sin6_addr.s6_addr,
-   tmp, 50);
-   if (str == NULL) {
-   strcpy(buffer, "inet_ntop failed\n");
-   error = 1;
-   goto getaddr_done;
-   }
-   if (offset == 0)
-   strcpy(buffer, tmp);
-   else
-   strcat(buffer, tmp);
-   strcat(buffer, ";");
-   offset += strlen(str) + 1;
-   if ((length - offset) < (ipv6_len + 1))
-   goto getaddr_done;
-
-   }
-
+   error = kvp_process_ip_address(curp->ifa_addr,
+   curp->ifa_addr->sa_family,
+   buffer,
+   length, );
+   if (error)
+   goto getaddr_done;
 
curp = curp->ifa_next;
}
-- 
1.7.4.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the 

[PATCH V2 06/18] Tools: hv: Further refactor kvp_get_ip_address()

[K. Y. Srinivasan]

In preparation for making kvp_get_ip_address() more generic, factor out
the code for handling IP addresses.

Signed-off-by: K. Y. Srinivasan k...@microsoft.com
Reviewed-by: Haiyang Zhang haiya...@microsoft.com
Reviewed-by: Olaf Hering o...@aepfle.de
Reviewed-by: Ben Hutchings b...@decadent.org.uk
---
 tools/hv/hv_kvp_daemon.c |   94 -
 1 files changed, 42 insertions(+), 52 deletions(-)

diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
index 3af37f0..3dc989f 100644
--- a/tools/hv/hv_kvp_daemon.c
+++ b/tools/hv/hv_kvp_daemon.c
@@ -491,17 +491,50 @@ done:
return;
 }
 
+static int kvp_process_ip_address(void *addrp,
+   int family, char *buffer,
+   int length,  int *offset)
+{
+   struct sockaddr_in *addr;
+   struct sockaddr_in6 *addr6;
+   int addr_length;
+   char tmp[50];
+   const char *str;
+
+   if (family == AF_INET) {
+   addr = (struct sockaddr_in *)addrp;
+   str = inet_ntop(family, addr-sin_addr, tmp, 50);
+   addr_length = INET_ADDRSTRLEN;
+   } else {
+   addr6 = (struct sockaddr_in6 *)addrp;
+   str = inet_ntop(family, addr6-sin6_addr.s6_addr, tmp, 50);
+   addr_length = INET6_ADDRSTRLEN;
+   }
+
+   if ((length - *offset)  addr_length + 1)
+   return 1;
+   if (str == NULL) {
+   strcpy(buffer, inet_ntop failed\n);
+   return 1;
+   }
+   if (*offset == 0)
+   strcpy(buffer, tmp);
+   else
+   strcat(buffer, tmp);
+   strcat(buffer, ;);
+
+   *offset += strlen(str) + 1;
+   return 0;
+}
+
 static int
 kvp_get_ip_address(int family, char *if_name, int op,
 void  *out_buffer, int length)
 {
struct ifaddrs *ifap;
struct ifaddrs *curp;
-   int ipv4_len = strlen(255.255.255.255) + 1;
-   int ipv6_len = strlen(:::::::)+1;
int offset = 0;
const char *str;
-   char tmp[50];
int error = 0;
char *buffer;
struct hv_kvp_ipaddr_value *ip_buffer;
@@ -556,55 +589,12 @@ kvp_get_ip_address(int family, char *if_name, int op,
continue;
}
 
-   if ((curp-ifa_addr-sa_family == AF_INET) 
-   ((family == AF_INET) || (family == 0))) {
-   struct sockaddr_in *addr =
-   (struct sockaddr_in *) curp-ifa_addr;
-
-   str = inet_ntop(AF_INET, addr-sin_addr, tmp, 50);
-   if (str == NULL) {
-   strcpy(buffer, inet_ntop failed\n);
-   error = 1;
-   goto getaddr_done;
-   }
-   if (offset == 0)
-   strcpy(buffer, tmp);
-   else
-   strcat(buffer, tmp);
-   strcat(buffer, ;);
-
-   offset += strlen(str) + 1;
-   if ((length - offset)  (ipv4_len + 1))
-   goto getaddr_done;
-
-   } else if ((family == AF_INET6) || (family == 0)) {
-
-   /*
-* We only support AF_INET and AF_INET6
-* and the list of addresses is separated by a ;.
-*/
-   struct sockaddr_in6 *addr =
-   (struct sockaddr_in6 *) curp-ifa_addr;
-
-   str = inet_ntop(AF_INET6,
-   addr-sin6_addr.s6_addr,
-   tmp, 50);
-   if (str == NULL) {
-   strcpy(buffer, inet_ntop failed\n);
-   error = 1;
-   goto getaddr_done;
-   }
-   if (offset == 0)
-   strcpy(buffer, tmp);
-   else
-   strcat(buffer, tmp);
-   strcat(buffer, ;);
-   offset += strlen(str) + 1;
-   if ((length - offset)  (ipv6_len + 1))
-   goto getaddr_done;
-
-   }
-
+   error = kvp_process_ip_address(curp-ifa_addr,
+   curp-ifa_addr-sa_family,
+   buffer,
+   length, offset);
+   if (error)
+   goto getaddr_done;
 
curp = curp-ifa_next;
}
-- 
1.7.4.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info 

Re: [PATCH V2 06/18] Tools: hv: Further refactor kvp_get_ip_address()

[Ben Hutchings]

On Mon, 2012-08-13 at 10:06 -0700, K. Y. Srinivasan wrote:
 In preparation for making kvp_get_ip_address() more generic, factor out
 the code for handling IP addresses.
 
 Signed-off-by: K. Y. Srinivasan k...@microsoft.com
 Reviewed-by: Haiyang Zhang haiya...@microsoft.com
 Reviewed-by: Olaf Hering o...@aepfle.de
 Reviewed-by: Ben Hutchings b...@decadent.org.uk

I looked at your last patch set, so in a sense these have been reviewed
by me.  But the 'Reviewed-by' line in a commit message means the
reviewer thinks it's OK; the reviewer must say that, and I didn't.

Ben.

 ---
  tools/hv/hv_kvp_daemon.c |   94 -
  1 files changed, 42 insertions(+), 52 deletions(-)
 
 diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
 index 3af37f0..3dc989f 100644
 --- a/tools/hv/hv_kvp_daemon.c
 +++ b/tools/hv/hv_kvp_daemon.c
 @@ -491,17 +491,50 @@ done:
   return;
  }
  
 +static int kvp_process_ip_address(void *addrp,
 + int family, char *buffer,
 + int length,  int *offset)
 +{
 + struct sockaddr_in *addr;
 + struct sockaddr_in6 *addr6;
 + int addr_length;
 + char tmp[50];
 + const char *str;
 +
 + if (family == AF_INET) {
 + addr = (struct sockaddr_in *)addrp;
 + str = inet_ntop(family, addr-sin_addr, tmp, 50);
 + addr_length = INET_ADDRSTRLEN;
 + } else {
 + addr6 = (struct sockaddr_in6 *)addrp;
 + str = inet_ntop(family, addr6-sin6_addr.s6_addr, tmp, 50);
 + addr_length = INET6_ADDRSTRLEN;
 + }
 +
 + if ((length - *offset)  addr_length + 1)
 + return 1;
 + if (str == NULL) {
 + strcpy(buffer, inet_ntop failed\n);
 + return 1;
 + }
 + if (*offset == 0)
 + strcpy(buffer, tmp);
 + else
 + strcat(buffer, tmp);
 + strcat(buffer, ;);
 +
 + *offset += strlen(str) + 1;
 + return 0;
 +}
 +
  static int
  kvp_get_ip_address(int family, char *if_name, int op,
void  *out_buffer, int length)
  {
   struct ifaddrs *ifap;
   struct ifaddrs *curp;
 - int ipv4_len = strlen(255.255.255.255) + 1;
 - int ipv6_len = strlen(:::::::)+1;
   int offset = 0;
   const char *str;
 - char tmp[50];
   int error = 0;
   char *buffer;
   struct hv_kvp_ipaddr_value *ip_buffer;
 @@ -556,55 +589,12 @@ kvp_get_ip_address(int family, char *if_name, int op,
   continue;
   }
  
 - if ((curp-ifa_addr-sa_family == AF_INET) 
 - ((family == AF_INET) || (family == 0))) {
 - struct sockaddr_in *addr =
 - (struct sockaddr_in *) curp-ifa_addr;
 -
 - str = inet_ntop(AF_INET, addr-sin_addr, tmp, 50);
 - if (str == NULL) {
 - strcpy(buffer, inet_ntop failed\n);
 - error = 1;
 - goto getaddr_done;
 - }
 - if (offset == 0)
 - strcpy(buffer, tmp);
 - else
 - strcat(buffer, tmp);
 - strcat(buffer, ;);
 -
 - offset += strlen(str) + 1;
 - if ((length - offset)  (ipv4_len + 1))
 - goto getaddr_done;
 -
 - } else if ((family == AF_INET6) || (family == 0)) {
 -
 - /*
 -  * We only support AF_INET and AF_INET6
 -  * and the list of addresses is separated by a ;.
 -  */
 - struct sockaddr_in6 *addr =
 - (struct sockaddr_in6 *) curp-ifa_addr;
 -
 - str = inet_ntop(AF_INET6,
 - addr-sin6_addr.s6_addr,
 - tmp, 50);
 - if (str == NULL) {
 - strcpy(buffer, inet_ntop failed\n);
 - error = 1;
 - goto getaddr_done;
 - }
 - if (offset == 0)
 - strcpy(buffer, tmp);
 - else
 - strcat(buffer, tmp);
 - strcat(buffer, ;);
 - offset += strlen(str) + 1;
 - if ((length - offset)  (ipv6_len + 1))
 - goto getaddr_done;
 -
 - }
 -
 + error = kvp_process_ip_address(curp-ifa_addr,
 + curp-ifa_addr-sa_family,
 + buffer,
 + length, offset);
 + if (error)
 + goto 

RE: [PATCH V2 06/18] Tools: hv: Further refactor kvp_get_ip_address()

[KY Srinivasan]

 -Original Message-
 From: Ben Hutchings [mailto:b...@decadent.org.uk]
 Sent: Monday, August 13, 2012 9:46 PM
 To: KY Srinivasan
 Cc: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
 de...@linuxdriverproject.org; virtualizat...@lists.osdl.org; o...@aepfle.de;
 a...@canonical.com
 Subject: Re: [PATCH V2 06/18] Tools: hv: Further refactor kvp_get_ip_address()
 
 On Mon, 2012-08-13 at 10:06 -0700, K. Y. Srinivasan wrote:
  In preparation for making kvp_get_ip_address() more generic, factor out
  the code for handling IP addresses.
 
  Signed-off-by: K. Y. Srinivasan k...@microsoft.com
  Reviewed-by: Haiyang Zhang haiya...@microsoft.com
  Reviewed-by: Olaf Hering o...@aepfle.de
  Reviewed-by: Ben Hutchings b...@decadent.org.uk
 
 I looked at your last patch set, so in a sense these have been reviewed
 by me.  But the 'Reviewed-by' line in a commit message means the
 reviewer thinks it's OK; the reviewer must say that, and I didn't.

My mistake, sorry about that.

Regards,

K. Y