Looks good. Thanks.
--Justin
On Dec 18, 2012, at 1:51 PM, Ben Pfaff <b...@nicira.com> wrote:
> When HMAP_FOR_EACH completes, the value in the loop control variable is not
> necessarily NULL. It is NULL minus the offset of the hmap_node struct
> member, which is nonnull if that offset is nonzero. Currently,
> 'all_ofproto_dpifs_node' is the first member in struct ofproto_dpif, so
> there is no real bug, but there would be if the struct were rearranged.
>
> This commit heads off the problem by avoiding any assumption about the
> loop control variable after HMAP_FOR_EACH.
>
> Signed-off-by: Ben Pfaff <b...@nicira.com>
> ---
> ofproto/ofproto-dpif.c | 24 ++++++++++++++++--------
> 1 files changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index ca0a065..95fd54e 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -794,6 +794,20 @@ port_open_type(const char *datapath_type, const char
> *port_type)
>
> /* Type functions. */
>
> +static struct ofproto_dpif *
> +lookup_ofproto_dpif_by_port_name(const char *name)
> +{
> + struct ofproto_dpif *ofproto;
> +
> + HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_node, &all_ofproto_dpifs) {
> + if (sset_contains(&ofproto->ports, name)) {
> + return ofproto;
> + }
> + }
> +
> + return NULL;
> +}
> +
> static int
> type_run(const char *type)
> {
> @@ -817,7 +831,7 @@ type_run(const char *type)
>
> /* Check for port changes in the dpif. */
> while ((error = dpif_port_poll(backer->dpif, &devname)) == 0) {
> - struct ofproto_dpif *ofproto = NULL;
> + struct ofproto_dpif *ofproto;
> struct dpif_port port;
>
> /* Don't report on the datapath's device. */
> @@ -825,13 +839,7 @@ type_run(const char *type)
> continue;
> }
>
> - HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_node,
> - &all_ofproto_dpifs) {
> - if (sset_contains(&ofproto->ports, devname)) {
> - break;
> - }
> - }
> -
> + ofproto = lookup_ofproto_dpif_by_port_name(devname);
> if (dpif_port_query_by_name(backer->dpif, devname, &port)) {
> /* The port was removed. If we know the datapath,
> * report it through poll_set(). If we don't, it may be
> --
> 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
