Hi Daniele,
One minor comment inline.
Cheers,
Mark
>
>The interface will be more similar to the cmap.
>
>Signed-off-by: Daniele Di Proietto <diproiet...@vmware.com>
>---
> lib/hmap.c | 26 ++++++++++++--------------
> lib/hmap.h | 7 ++++++-
> lib/sset.c | 12 +++++-------
> lib/sset.h | 7 ++++++-
> ofproto/ofproto-dpif.c | 8 +++-----
> 5 files changed, 32 insertions(+), 28 deletions(-)
>
>diff --git a/lib/hmap.c b/lib/hmap.c
>index b70ce51..9462c5e 100644
>--- a/lib/hmap.c
>+++ b/lib/hmap.c
>@@ -236,24 +236,22 @@ hmap_random_node(const struct hmap *hmap)
> }
>
> /* Returns the next node in 'hmap' in hash order, or NULL if no nodes remain
> in
>- * 'hmap'. Uses '*bucketp' and '*offsetp' to determine where to begin
>- * iteration, and stores new values to pass on the next iteration into them
>- * before returning.
>+ * 'hmap'. Uses '*pos' to determine where to begin iteration, and updates
>+ * '*pos' to pass on the next iteration into them before returning.
> *
> * It's better to use plain HMAP_FOR_EACH and related functions, since they
> are
> * faster and better at dealing with hmaps that change during iteration.
> *
>- * Before beginning iteration, store 0 into '*bucketp' and '*offsetp'.
>- */
>+ * Before beginning iteration, set '*pos' to all zeros. */
> struct hmap_node *
> hmap_at_position(const struct hmap *hmap,
>- uint32_t *bucketp, uint32_t *offsetp)
>+ struct hmap_position *pos)
> {
> size_t offset;
> size_t b_idx;
>
>- offset = *offsetp;
>- for (b_idx = *bucketp; b_idx <= hmap->mask; b_idx++) {
>+ offset = pos->offset;
>+ for (b_idx = pos->bucket; b_idx <= hmap->mask; b_idx++) {
> struct hmap_node *node;
> size_t n_idx;
>
>@@ -261,11 +259,11 @@ hmap_at_position(const struct hmap *hmap,
> n_idx++, node = node->next) {
> if (n_idx == offset) {
> if (node->next) {
>- *bucketp = node->hash & hmap->mask;
>- *offsetp = offset + 1;
>+ pos->bucket = node->hash & hmap->mask;
>+ pos->offset = offset + 1;
> } else {
>- *bucketp = (node->hash & hmap->mask) + 1;
>- *offsetp = 0;
>+ pos->bucket = (node->hash & hmap->mask) + 1;
>+ pos->offset = 0;
> }
> return node;
> }
>@@ -273,8 +271,8 @@ hmap_at_position(const struct hmap *hmap,
> offset = 0;
> }
>
>- *bucketp = 0;
>- *offsetp = 0;
>+ pos->bucket = 0;
>+ pos->offset = 0;
> return NULL;
> }
>
>diff --git a/lib/hmap.h b/lib/hmap.h
>index 08c4719..9a96c5f 100644
>--- a/lib/hmap.h
>+++ b/lib/hmap.h
>@@ -201,8 +201,13 @@ static inline struct hmap_node *hmap_first(const struct
>hmap *);
> static inline struct hmap_node *hmap_next(const struct hmap *,
> const struct hmap_node *);
>
>+struct hmap_position {
>+ unsigned int bucket;
>+ unsigned int offset;
>+};
>+
> struct hmap_node *hmap_at_position(const struct hmap *,
>- uint32_t *bucket, uint32_t *offset);
>+ struct hmap_position *);
>
> /* Returns the number of nodes currently in 'hmap'. */
> static inline size_t
>diff --git a/lib/sset.c b/lib/sset.c
>index f9d4fc0..4fd3fae 100644
>--- a/lib/sset.c
>+++ b/lib/sset.c
>@@ -251,21 +251,19 @@ sset_equals(const struct sset *a, const struct sset *b)
> }
>
> /* Returns the next node in 'set' in hash order, or NULL if no nodes remain in
>- * 'set'. Uses '*bucketp' and '*offsetp' to determine where to begin
>- * iteration, and stores new values to pass on the next iteration into them
>- * before returning.
>+ * 'set'. Uses '*pos' to determine where to begin iteration, and updates
>+ * '*pos' to pass on the next iteration into them before returning.
> *
> * It's better to use plain SSET_FOR_EACH and related functions, since they
> are
> * faster and better at dealing with ssets that change during iteration.
> *
>- * Before beginning iteration, store 0 into '*bucketp' and '*offsetp'.
>- */
>+ * Before beginning iteration, set '*pos' to all zeros. */
> struct sset_node *
>-sset_at_position(const struct sset *set, uint32_t *bucketp, uint32_t *offsetp)
>+sset_at_position(const struct sset *set, struct sset_position *pos)
> {
> struct hmap_node *hmap_node;
>
>- hmap_node = hmap_at_position(&set->map, bucketp, offsetp);
>+ hmap_node = hmap_at_position(&set->map, &pos->pos);
> return SSET_NODE_FROM_HMAP_NODE(hmap_node);
> }
>
>diff --git a/lib/sset.h b/lib/sset.h
>index 7d1d496..9c2f703 100644
>--- a/lib/sset.h
>+++ b/lib/sset.h
>@@ -64,8 +64,13 @@ char *sset_pop(struct sset *);
> struct sset_node *sset_find(const struct sset *, const char *);
> bool sset_contains(const struct sset *, const char *);
> bool sset_equals(const struct sset *, const struct sset *);
>+
>+struct sset_position {
>+ struct hmap_position pos;
>+};
[MK] Would a typedef be more appropriate here, as it would avoid the additional
layer of dereference?
>+
> struct sset_node *sset_at_position(const struct sset *,
>- uint32_t *bucketp, uint32_t *offsetp);
>+ struct sset_position *);
>
> /* Set operations. */
> void sset_intersect(struct sset *, const struct sset *);
>diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
>index 530c49a..aceb11f 100644
>--- a/ofproto/ofproto-dpif.c
>+++ b/ofproto/ofproto-dpif.c
>@@ -3558,8 +3558,7 @@ port_get_lacp_stats(const struct ofport *ofport_, struct
>lacp_slave_stats *stats
> }
>
> struct port_dump_state {
>- uint32_t bucket;
>- uint32_t offset;
>+ struct sset_position pos;
> bool ghost;
>
> struct ofproto_port port;
>@@ -3587,7 +3586,7 @@ port_dump_next(const struct ofproto *ofproto_, void
>*state_,
> state->has_port = false;
> }
> sset = state->ghost ? &ofproto->ghost_ports : &ofproto->ports;
>- while ((node = sset_at_position(sset, &state->bucket, &state->offset))) {
>+ while ((node = sset_at_position(sset, &state->pos))) {
> int error;
>
> error = port_query_by_name(ofproto_, node->name, &state->port);
>@@ -3602,8 +3601,7 @@ port_dump_next(const struct ofproto *ofproto_, void
>*state_,
>
> if (!state->ghost) {
> state->ghost = true;
>- state->bucket = 0;
>- state->offset = 0;
>+ memset(&state->pos, 0, sizeof state->pos);
> return port_dump_next(ofproto_, state_, port);
> }
>
>--
>2.1.4
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
