Looks good to me
On Thu, Aug 1, 2013 at 2:29 PM, Ben Pfaff <b...@nicira.com> wrote:
> When threading comes into the picture there arises the possibility of a
> race between netdev_vport_patch_peer()'s caller using the returned string
> and another caller changing the peer. It is safer to return a copy.
>
> Signed-off-by: Ben Pfaff <b...@nicira.com>
> ---
> lib/netdev-vport.c | 21 ++++++++++++++++-----
> lib/netdev-vport.h | 2 +-
> ofproto/ofproto-dpif.c | 8 +++++---
> 3 files changed, 22 insertions(+), 9 deletions(-)
>
> diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
> index ac3da63..287edae 100644
> --- a/lib/netdev-vport.c
> +++ b/lib/netdev-vport.c
> @@ -553,12 +553,23 @@ get_tunnel_config(const struct netdev *dev, struct
> smap *args)
>
> /* Code specific to patch ports. */
>
> -const char *
> -netdev_vport_patch_peer(const struct netdev *netdev)
> +/* If 'netdev' is a patch port, returns the name of its peer as a
> malloc()'d
> + * string that the caller must free.
> + *
> + * If 'netdev' is not a patch port, returns NULL. */
> +char *
> +netdev_vport_patch_peer(const struct netdev *netdev_)
> {
> - return (netdev_vport_is_patch(netdev)
> - ? netdev_vport_cast(netdev)->peer
> - : NULL);
> + char *peer = NULL;
> +
> + if (netdev_vport_is_patch(netdev_)) {
> + struct netdev_vport *netdev = netdev_vport_cast(netdev_);
> + if (netdev->peer) {
> + peer = xstrdup(netdev->peer);
> + }
> + }
> +
> + return peer;
> }
>
> void
> diff --git a/lib/netdev-vport.h b/lib/netdev-vport.h
> index 5394966..dc49097 100644
> --- a/lib/netdev-vport.h
> +++ b/lib/netdev-vport.h
> @@ -31,7 +31,7 @@ void netdev_vport_patch_register(void);
>
> bool netdev_vport_is_patch(const struct netdev *);
>
> -const char *netdev_vport_patch_peer(const struct netdev *netdev);
> +char *netdev_vport_patch_peer(const struct netdev *netdev);
>
> void netdev_vport_inc_rx(const struct netdev *,
> const struct dpif_flow_stats *);
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 839de69..9d53ccf 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -2890,7 +2890,7 @@ ofport_update_peer(struct ofport_dpif *ofport)
> {
> const struct ofproto_dpif *ofproto;
> struct dpif_backer *backer;
> - const char *peer_name;
> + char *peer_name;
>
> if (!netdev_vport_is_patch(ofport->up.netdev)) {
> return;
> @@ -2912,7 +2912,7 @@ ofport_update_peer(struct ofport_dpif *ofport)
> HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_node, &all_ofproto_dpifs) {
> struct ofport *peer_ofport;
> struct ofport_dpif *peer;
> - const char *peer_peer;
> + char *peer_peer;
>
> if (ofproto->backer != backer) {
> continue;
> @@ -2930,9 +2930,11 @@ ofport_update_peer(struct ofport_dpif *ofport)
> ofport->peer = peer;
> ofport->peer->peer = ofport;
> }
>
+ free(peer_peer);
>
> - return;
> + break;
> }
> + free(peer_name);
> }
>
> static void
> --
> 1.7.10.4
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
