<div dir="ltr">Looks good.  Thanks. <br></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, Jun 5, 2013 at 1:05 PM, Ben Pfaff <span dir="ltr">&lt;<a href="mailto:blp@nicira.com" target="_blank">blp@nicira.com</a>&gt;</span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Returning a static data buffer makes code more brittle and definitely<br>
not thread-safe, so this commit switches to using a caller-provided<br>
buffer instead.<br>
<br>
Signed-off-by: Ben Pfaff &lt;<a href="mailto:blp@nicira.com">blp@nicira.com</a>&gt;<br>
---<br>
 lib/dpif-linux.c       |    4 +++-<br>
 lib/dpif-netdev.c      |    9 ++++++---<br>
 lib/netdev-vport.c     |   18 ++++++++++++++----<br>
 lib/netdev-vport.h     |    7 ++++++-<br>
 ofproto/ofproto-dpif.c |   17 +++++++++++++----<br>
 5 files changed, 42 insertions(+), 13 deletions(-)<br>
<br>
diff --git a/lib/dpif-linux.c b/lib/dpif-linux.c<br>
index ac20ae7..1383b58 100644<br>
--- a/lib/dpif-linux.c<br>
+++ b/lib/dpif-linux.c<br>
@@ -484,7 +484,9 @@ dpif_linux_port_add(struct dpif *dpif_, struct netdev *netdev,<br>
 {<br>
     struct dpif_linux *dpif = dpif_linux_cast(dpif_);<br>
     const struct netdev_tunnel_config *tnl_cfg;<br>
-    const char *name = netdev_vport_get_dpif_port(netdev);<br>
+    char namebuf[NETDEV_VPORT_NAME_BUFSIZE];<br>
+    const char *name = netdev_vport_get_dpif_port(netdev,<br>
+                                                  namebuf, sizeof namebuf);<br>
     const char *type = netdev_get_type(netdev);<br>
     struct dpif_linux_vport request, reply;<br>
     struct nl_sock *sock = NULL;<br>
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c<br>
index 9ea8d24..36992a6 100644<br>
--- a/lib/dpif-netdev.c<br>
+++ b/lib/dpif-netdev.c<br>
@@ -436,8 +436,11 @@ dpif_netdev_port_add(struct dpif *dpif, struct netdev *netdev,<br>
                      uint32_t *port_nop)<br>
 {<br>
     struct dp_netdev *dp = get_dp_netdev(dpif);<br>
+    char namebuf[NETDEV_VPORT_NAME_BUFSIZE];<br>
+    const char *dpif_port;<br>
     int port_no;<br>
<br>
+    dpif_port = netdev_vport_get_dpif_port(netdev, namebuf, sizeof namebuf);<br>
     if (*port_nop != UINT32_MAX) {<br>
         if (*port_nop &gt;= MAX_PORTS) {<br>
             return EFBIG;<br>
@@ -446,12 +449,11 @@ dpif_netdev_port_add(struct dpif *dpif, struct netdev *netdev,<br>
         }<br>
         port_no = *port_nop;<br>
     } else {<br>
-        port_no = choose_port(dp, netdev_vport_get_dpif_port(netdev));<br>
+        port_no = choose_port(dp, dpif_port);<br>
     }<br>
     if (port_no &gt;= 0) {<br>
         *port_nop = port_no;<br>
-        return do_add_port(dp, netdev_vport_get_dpif_port(netdev),<br>
-                           netdev_get_type(netdev), port_no);<br>
+        return do_add_port(dp, dpif_port, netdev_get_type(netdev), port_no);<br>
     }<br>
     return EFBIG;<br>
 }<br>
@@ -1074,6 +1076,7 @@ dpif_netdev_run(struct dpif *dpif)<br>
             dp_netdev_port_input(dp, port, &amp;packet);<br>
         } else if (error != EAGAIN &amp;&amp; error != EOPNOTSUPP) {<br>
             static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);<br>
+<br>
             VLOG_ERR_RL(&amp;rl, &quot;error receiving data from %s: %s&quot;,<br>
                         netdev_get_name(port-&gt;netdev), strerror(error));<br>
         }<br>
diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c<br>
index 4bb41bd..bdb0c4d 100644<br>
--- a/lib/netdev-vport.c<br>
+++ b/lib/netdev-vport.c<br>
@@ -121,12 +121,12 @@ netdev_vport_class_get_dpif_port(const struct netdev_class *class)<br>
 }<br>
<br>
 const char *<br>
-netdev_vport_get_dpif_port(const struct netdev *netdev)<br>
+netdev_vport_get_dpif_port(const struct netdev *netdev,<br>
+                           char namebuf[], size_t bufsize)<br>
 {<br>
     if (netdev_vport_needs_dst_port(netdev)) {<br>
         const struct netdev_vport *vport = netdev_vport_cast(netdev);<br>
         const char *type = netdev_get_type(netdev);<br>
-        static char dpif_port_combined[IFNAMSIZ];<br>
<br>
         /*<br>
          * Note: IFNAMSIZ is 16 bytes long. The maximum length of a VXLAN<br>
@@ -134,10 +134,11 @@ netdev_vport_get_dpif_port(const struct netdev *netdev)<br>
          * assert here on the size of strlen(type) in case that changes<br>
          * in the future.<br>
          */<br>
+        BUILD_ASSERT(NETDEV_VPORT_NAME_BUFSIZE &gt;= IFNAMSIZ);<br>
         ovs_assert(strlen(type) + 10 &lt; IFNAMSIZ);<br>
-        snprintf(dpif_port_combined, IFNAMSIZ, &quot;%s_sys_%d&quot;, type,<br>
+        snprintf(namebuf, bufsize, &quot;%s_sys_%d&quot;, type,<br>
                  ntohs(vport-&gt;tnl_cfg.dst_port));<br>
-        return dpif_port_combined;<br>
+        return namebuf;<br>
     } else {<br>
         const struct netdev_class *class = netdev_get_class(netdev);<br>
         const char *dpif_port = netdev_vport_class_get_dpif_port(class);<br>
@@ -145,6 +146,15 @@ netdev_vport_get_dpif_port(const struct netdev *netdev)<br>
     }<br>
 }<br>
<br>
+char *<br>
+netdev_vport_get_dpif_port_strdup(const struct netdev *netdev)<br>
+{<br>
+    char namebuf[NETDEV_VPORT_NAME_BUFSIZE];<br>
+<br>
+    return xstrdup(netdev_vport_get_dpif_port(netdev, namebuf,<br>
+                                              sizeof namebuf));<br>
+}<br>
+<br>
 static int<br>
 netdev_vport_create(const struct netdev_class *netdev_class, const char *name,<br>
                     struct netdev **netdevp)<br>
diff --git a/lib/netdev-vport.h b/lib/netdev-vport.h<br>
index 94fe937..5394966 100644<br>
--- a/lib/netdev-vport.h<br>
+++ b/lib/netdev-vport.h<br>
@@ -18,6 +18,7 @@<br>
 #define NETDEV_VPORT_H 1<br>
<br>
 #include &lt;stdbool.h&gt;<br>
+#include &lt;stddef.h&gt;<br>
<br>
 struct dpif_linux_vport;<br>
 struct dpif_flow_stats;<br>
@@ -37,7 +38,11 @@ void netdev_vport_inc_rx(const struct netdev *,<br>
 void netdev_vport_inc_tx(const struct netdev *,<br>
                          const struct dpif_flow_stats *);<br>
<br>
-const char *netdev_vport_get_dpif_port(const struct netdev *);<br>
 const char *netdev_vport_class_get_dpif_port(const struct netdev_class *);<br>
<br>
+enum { NETDEV_VPORT_NAME_BUFSIZE = 16 };<br>
+const char *netdev_vport_get_dpif_port(const struct netdev *,<br>
+                                       char namebuf[], size_t bufsize);<br>
+char *netdev_vport_get_dpif_port_strdup(const struct netdev *);<br>
+<br>
 #endif /* netdev-vport.h */<br>
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c<br>
index 096f0f2..cfcd4c8 100644<br>
--- a/ofproto/ofproto-dpif.c<br>
+++ b/ofproto/ofproto-dpif.c<br>
@@ -956,13 +956,15 @@ type_run(const char *type)<br>
             }<br>
<br>
             HMAP_FOR_EACH (iter, up.hmap_node, &amp;ofproto-&gt;up.ports) {<br>
+                char namebuf[NETDEV_VPORT_NAME_BUFSIZE];<br>
                 const char *dp_port;<br>
<br>
                 if (!iter-&gt;tnl_port) {<br>
                     continue;<br>
                 }<br>
<br>
-                dp_port = netdev_vport_get_dpif_port(iter-&gt;up.netdev);<br>
+                dp_port = netdev_vport_get_dpif_port(iter-&gt;up.netdev,<br>
+                                                     namebuf, sizeof namebuf);<br>
                 node = simap_find(&amp;tmp_backers, dp_port);<br>
                 if (node) {<br>
                     simap_put(&amp;backer-&gt;tnl_backers, dp_port, node-&gt;data);<br>
@@ -1806,6 +1808,7 @@ port_construct(struct ofport *port_)<br>
     struct ofport_dpif *port = ofport_dpif_cast(port_);<br>
     struct ofproto_dpif *ofproto = ofproto_dpif_cast(port-&gt;up.ofproto);<br>
     const struct netdev *netdev = port-&gt;up.netdev;<br>
+    char namebuf[NETDEV_VPORT_NAME_BUFSIZE];<br>
     struct dpif_port dpif_port;<br>
     int error;<br>
<br>
@@ -1834,7 +1837,8 @@ port_construct(struct ofport *port_)<br>
     }<br>
<br>
     error = dpif_port_query_by_name(ofproto-&gt;backer-&gt;dpif,<br>
-                                    netdev_vport_get_dpif_port(netdev),<br>
+                                    netdev_vport_get_dpif_port(netdev, namebuf,<br>
+                                                               sizeof namebuf),<br>
                                     &amp;dpif_port);<br>
     if (error) {<br>
         return error;<br>
@@ -1871,9 +1875,12 @@ port_destruct(struct ofport *port_)<br>
 {<br>
     struct ofport_dpif *port = ofport_dpif_cast(port_);<br>
     struct ofproto_dpif *ofproto = ofproto_dpif_cast(port-&gt;up.ofproto);<br>
-    const char *dp_port_name = netdev_vport_get_dpif_port(port-&gt;up.netdev);<br>
     const char *devname = netdev_get_name(port-&gt;up.netdev);<br>
+    char namebuf[NETDEV_VPORT_NAME_BUFSIZE];<br>
+    const char *dp_port_name;<br>
<br>
+    dp_port_name = netdev_vport_get_dpif_port(port-&gt;up.netdev, namebuf,<br>
+                                              sizeof namebuf);<br>
     if (dpif_port_exists(ofproto-&gt;backer-&gt;dpif, dp_port_name)) {<br>
         /* The underlying device is still there, so delete it.  This<br>
          * happens when the ofproto is being destroyed, since the caller<br>
@@ -3317,14 +3324,16 @@ static int<br>
 port_add(struct ofproto *ofproto_, struct netdev *netdev)<br>
 {<br>
     struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);<br>
-    const char *dp_port_name = netdev_vport_get_dpif_port(netdev);<br>
     const char *devname = netdev_get_name(netdev);<br>
+    char namebuf[NETDEV_VPORT_NAME_BUFSIZE];<br>
+    const char *dp_port_name;<br>
<br>
     if (netdev_vport_is_patch(netdev)) {<br>
         sset_add(&amp;ofproto-&gt;ghost_ports, netdev_get_name(netdev));<br>
         return 0;<br>
     }<br>
<br>
+    dp_port_name = netdev_vport_get_dpif_port(netdev, namebuf, sizeof namebuf);<br>
     if (!dpif_port_exists(ofproto-&gt;backer-&gt;dpif, dp_port_name)) {<br>
         uint32_t port_no = UINT32_MAX;<br>
         int error;<br>
<span class="HOEnZb"><font color="#888888">--<br>
1.7.2.5<br>
<br>
_______________________________________________<br>
dev mailing list<br>
<a href="mailto:dev@openvswitch.org">dev@openvswitch.org</a><br>
<a href="http://openvswitch.org/mailman/listinfo/dev" target="_blank">http://openvswitch.org/mailman/listinfo/dev</a><br>
</font></span></blockquote></div><br></div>