[ovs-dev,IPv6,v2,05/10] ovn-util: Preallocate address strings.
diff mbox

Message ID 1469773580-33112-5-git-send-email-jpettit@ovn.org
State Accepted
Headers show

Commit Message

Justin Pettit July 29, 2016, 6:26 a.m. UTC
Suggested-by: Ben Pfaff <blp@ovn.org>
Signed-off-by: Justin Pettit <jpettit@ovn.org>
---
 ovn/lib/ovn-util.c | 29 ++++++++---------------------
 ovn/lib/ovn-util.h | 14 +++++++-------
 2 files changed, 15 insertions(+), 28 deletions(-)

Comments

Ben Pfaff July 29, 2016, 4:58 p.m. UTC | #1
On Thu, Jul 28, 2016 at 11:26:15PM -0700, Justin Pettit wrote:
> Suggested-by: Ben Pfaff <blp@ovn.org>
> Signed-off-by: Justin Pettit <jpettit@ovn.org>

> +    ovs_be32 bcast = addr | ~na->mask;
> +    inet_ntop(AF_INET, &addr, na->addr_s, INET_ADDRSTRLEN);
> +    inet_ntop(AF_INET, &na->network, na->network_s, INET_ADDRSTRLEN);
> +    inet_ntop(AF_INET, &bcast, na->bcast_s, INET_ADDRSTRLEN);

I'd use sizeof na->addr_s (etc.) above, instead of INET_ADDRSTRLEN.

>  }
>  
>  static void
> @@ -55,11 +56,8 @@ add_ipv6_netaddr(struct lport_addresses *laddrs, struct in6_addr addr,
>      na->plen = plen;
>      in6_addr_solicited_node(&na->sn_addr, &addr);
>  
> -    na->addr_s = xmalloc(INET6_ADDRSTRLEN);
>      inet_ntop(AF_INET6, &addr, na->addr_s, INET6_ADDRSTRLEN);
> -    na->sn_addr_s = xmalloc(INET6_ADDRSTRLEN);
>      inet_ntop(AF_INET6, &na->sn_addr, na->sn_addr_s, INET6_ADDRSTRLEN);
> -    na->network_s = xmalloc(INET6_ADDRSTRLEN);
>      inet_ntop(AF_INET6, &na->network, na->network_s, INET6_ADDRSTRLEN);

Similarly here.

Thanks,

Ben.
Justin Pettit July 29, 2016, 5:29 p.m. UTC | #2
> On Jul 29, 2016, at 11:58 AM, Ben Pfaff <blp@ovn.org> wrote:
> 
>> On Thu, Jul 28, 2016 at 11:26:15PM -0700, Justin Pettit wrote:
>> Suggested-by: Ben Pfaff <blp@ovn.org>
>> Signed-off-by: Justin Pettit <jpettit@ovn.org>
> 
>> +    ovs_be32 bcast = addr | ~na->mask;
>> +    inet_ntop(AF_INET, &addr, na->addr_s, INET_ADDRSTRLEN);
>> +    inet_ntop(AF_INET, &na->network, na->network_s, INET_ADDRSTRLEN);
>> +    inet_ntop(AF_INET, &bcast, na->bcast_s, INET_ADDRSTRLEN);
> 
> I'd use sizeof na->addr_s (etc.) above, instead of INET_ADDRSTRLEN.
> 
>> }
>> 
>> static void
>> @@ -55,11 +56,8 @@ add_ipv6_netaddr(struct lport_addresses *laddrs, struct in6_addr addr,
>>     na->plen = plen;
>>     in6_addr_solicited_node(&na->sn_addr, &addr);
>> 
>> -    na->addr_s = xmalloc(INET6_ADDRSTRLEN);
>>     inet_ntop(AF_INET6, &addr, na->addr_s, INET6_ADDRSTRLEN);
>> -    na->sn_addr_s = xmalloc(INET6_ADDRSTRLEN);
>>     inet_ntop(AF_INET6, &na->sn_addr, na->sn_addr_s, INET6_ADDRSTRLEN);
>> -    na->network_s = xmalloc(INET6_ADDRSTRLEN);
>>     inet_ntop(AF_INET6, &na->network, na->network_s, INET6_ADDRSTRLEN);
> 
> Similarly here.

Will do. Did you want to see a revision or are you okay acking it?

--Justin
Ben Pfaff July 29, 2016, 5:55 p.m. UTC | #3
On Fri, Jul 29, 2016 at 12:29:16PM -0500, Justin Pettit wrote:
> 
> > On Jul 29, 2016, at 11:58 AM, Ben Pfaff <blp@ovn.org> wrote:
> > 
> >> On Thu, Jul 28, 2016 at 11:26:15PM -0700, Justin Pettit wrote:
> >> Suggested-by: Ben Pfaff <blp@ovn.org>
> >> Signed-off-by: Justin Pettit <jpettit@ovn.org>
> > 
> >> +    ovs_be32 bcast = addr | ~na->mask;
> >> +    inet_ntop(AF_INET, &addr, na->addr_s, INET_ADDRSTRLEN);
> >> +    inet_ntop(AF_INET, &na->network, na->network_s, INET_ADDRSTRLEN);
> >> +    inet_ntop(AF_INET, &bcast, na->bcast_s, INET_ADDRSTRLEN);
> > 
> > I'd use sizeof na->addr_s (etc.) above, instead of INET_ADDRSTRLEN.
> > 
> >> }
> >> 
> >> static void
> >> @@ -55,11 +56,8 @@ add_ipv6_netaddr(struct lport_addresses *laddrs, struct in6_addr addr,
> >>     na->plen = plen;
> >>     in6_addr_solicited_node(&na->sn_addr, &addr);
> >> 
> >> -    na->addr_s = xmalloc(INET6_ADDRSTRLEN);
> >>     inet_ntop(AF_INET6, &addr, na->addr_s, INET6_ADDRSTRLEN);
> >> -    na->sn_addr_s = xmalloc(INET6_ADDRSTRLEN);
> >>     inet_ntop(AF_INET6, &na->sn_addr, na->sn_addr_s, INET6_ADDRSTRLEN);
> >> -    na->network_s = xmalloc(INET6_ADDRSTRLEN);
> >>     inet_ntop(AF_INET6, &na->network, na->network_s, INET6_ADDRSTRLEN);
> > 
> > Similarly here.
> 
> Will do. Did you want to see a revision or are you okay acking it?

I thought I'd acked it.

Acked-by: Ben Pfaff <blp@ovn.org>

Patch
diff mbox

diff --git a/ovn/lib/ovn-util.c b/ovn/lib/ovn-util.c
index de54624..51e3026 100644
--- a/ovn/lib/ovn-util.c
+++ b/ovn/lib/ovn-util.c
@@ -34,9 +34,10 @@  add_ipv4_netaddr(struct lport_addresses *laddrs, ovs_be32 addr,
     na->network = addr & na->mask;
     na->plen = plen;
 
-    na->addr_s = xasprintf(IP_FMT, IP_ARGS(addr));
-    na->network_s = xasprintf(IP_FMT, IP_ARGS(na->network));
-    na->bcast_s = xasprintf(IP_FMT, IP_ARGS(addr | ~na->mask));
+    ovs_be32 bcast = addr | ~na->mask;
+    inet_ntop(AF_INET, &addr, na->addr_s, INET_ADDRSTRLEN);
+    inet_ntop(AF_INET, &na->network, na->network_s, INET_ADDRSTRLEN);
+    inet_ntop(AF_INET, &bcast, na->bcast_s, INET_ADDRSTRLEN);
 }
 
 static void
@@ -55,11 +56,8 @@  add_ipv6_netaddr(struct lport_addresses *laddrs, struct in6_addr addr,
     na->plen = plen;
     in6_addr_solicited_node(&na->sn_addr, &addr);
 
-    na->addr_s = xmalloc(INET6_ADDRSTRLEN);
     inet_ntop(AF_INET6, &addr, na->addr_s, INET6_ADDRSTRLEN);
-    na->sn_addr_s = xmalloc(INET6_ADDRSTRLEN);
     inet_ntop(AF_INET6, &na->sn_addr, na->sn_addr_s, INET6_ADDRSTRLEN);
-    na->network_s = xmalloc(INET6_ADDRSTRLEN);
     inet_ntop(AF_INET6, &na->network, na->network_s, INET6_ADDRSTRLEN);
 }
 
@@ -85,7 +83,8 @@  extract_lsp_addresses(char *address, struct lport_addresses *laddrs)
         return false;
     }
 
-    laddrs->ea_s = xasprintf(ETH_ADDR_FMT, ETH_ADDR_ARGS(laddrs->ea));
+    snprintf(laddrs->ea_s, sizeof laddrs->ea_s, ETH_ADDR_FMT,
+             ETH_ADDR_ARGS(laddrs->ea));
 
     ovs_be32 ip4;
     struct in6_addr ip6;
@@ -138,7 +137,8 @@  extract_lrp_networks(const struct nbrec_logical_router_port *lrp,
         laddrs->ea = eth_addr_zero;
         return false;
     }
-    laddrs->ea_s = xasprintf(ETH_ADDR_FMT, ETH_ADDR_ARGS(laddrs->ea));
+    snprintf(laddrs->ea_s, sizeof laddrs->ea_s, ETH_ADDR_FMT,
+             ETH_ADDR_ARGS(laddrs->ea));
 
     for (int i = 0; i < lrp->n_networks; i++) {
         ovs_be32 ip4;
@@ -181,20 +181,7 @@  extract_lrp_networks(const struct nbrec_logical_router_port *lrp,
 void
 destroy_lport_addresses(struct lport_addresses *laddrs)
 {
-    free(laddrs->ea_s);
-
-    for (int i = 0; i < laddrs->n_ipv4_addrs; i++) {
-        free(laddrs->ipv4_addrs[i].addr_s);
-        free(laddrs->ipv4_addrs[i].network_s);
-        free(laddrs->ipv4_addrs[i].bcast_s);
-    }
     free(laddrs->ipv4_addrs);
-
-    for (int i = 0; i < laddrs->n_ipv6_addrs; i++) {
-        free(laddrs->ipv6_addrs[i].addr_s);
-        free(laddrs->ipv6_addrs[i].sn_addr_s);
-        free(laddrs->ipv6_addrs[i].network_s);
-    }
     free(laddrs->ipv6_addrs);
 }
 
diff --git a/ovn/lib/ovn-util.h b/ovn/lib/ovn-util.h
index e9f3ec2..97d9483 100644
--- a/ovn/lib/ovn-util.h
+++ b/ovn/lib/ovn-util.h
@@ -26,9 +26,9 @@  struct ipv4_netaddr {
     ovs_be32 network;         /* 192.168.10.0 */
     unsigned int plen;        /* CIDR Prefix: 24. */
 
-    char *addr_s;             /* "192.168.10.123" */
-    char *network_s;          /* "192.168.10.0" */
-    char *bcast_s;            /* "192.168.10.255" */
+    char addr_s[INET_ADDRSTRLEN + 1];     /* "192.168.10.123" */
+    char network_s[INET_ADDRSTRLEN + 1];  /* "192.168.10.0" */
+    char bcast_s[INET_ADDRSTRLEN + 1];    /* "192.168.10.255" */
 };
 
 struct ipv6_netaddr {
@@ -38,13 +38,13 @@  struct ipv6_netaddr {
     struct in6_addr network;  /* fc00:: */
     unsigned int plen;        /* CIDR Prefix: 64 */
 
-    char *addr_s;             /* "fc00::1" */
-    char *sn_addr_s;          /* "ff02:1:ff00::1" */
-    char *network_s;          /* "fc00::" */
+    char addr_s[INET6_ADDRSTRLEN + 1];    /* "fc00::1" */
+    char sn_addr_s[INET6_ADDRSTRLEN + 1]; /* "ff02:1:ff00::1" */
+    char network_s[INET6_ADDRSTRLEN + 1]; /* "fc00::" */
 };
 
 struct lport_addresses {
-    char *ea_s;
+    char ea_s[ETH_ADDR_STRLEN + 1];
     struct eth_addr ea;
     size_t n_ipv4_addrs;
     struct ipv4_netaddr *ipv4_addrs;