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