list: use short version of safe loops if possible.

Using the SHORT version of the *_SAFE loops makes the code cleaner
and less error-prone. So, use the SHORT version and remove the extra
variable when possible.

In order to be able to use both long and short versions without changing
the name of the macro for all the clients, overload the existing name
and select the appropriate version depending on the number of arguments.

Acked-by: Dumitru Ceara <[email protected]>
Acked-by: Eelco Chaudron <[email protected]>
Signed-off-by: Adrian Moreno <[email protected]>
Signed-off-by: Ilya Maximets <[email protected]>

include/openvswitch/list.h

@@ -92,7 +92,8 @@ static inline bool ovs_list_is_short(const struct ovs_list *);
          CONDITION_MULTIVAR(VAR, MEMBER, ITER_VAR(VAR) != (LIST));            \
          UPDATE_MULTIVAR(VAR, ITER_VAR(VAR)->prev))
 
-#define LIST_FOR_EACH_REVERSE_SAFE(VAR, PREV, MEMBER, LIST)                   \
+/* LONG version of SAFE iterators. */
+#define LIST_FOR_EACH_REVERSE_SAFE_LONG(VAR, PREV, MEMBER, LIST)              \
     for (INIT_MULTIVAR_SAFE_LONG(VAR, PREV, MEMBER, (LIST)->prev,             \
                                  struct ovs_list);                            \
          CONDITION_MULTIVAR_SAFE_LONG(VAR, PREV, MEMBER,                      \
@@ -101,7 +102,7 @@ static inline bool ovs_list_is_short(const struct ovs_list *);
                                       ITER_VAR(PREV) != (LIST));              \
          UPDATE_MULTIVAR_SAFE_LONG(VAR, PREV))
 
-#define LIST_FOR_EACH_SAFE(VAR, NEXT, MEMBER, LIST)                           \
+#define LIST_FOR_EACH_SAFE_LONG(VAR, NEXT, MEMBER, LIST)                      \
     for (INIT_MULTIVAR_SAFE_LONG(VAR, NEXT, MEMBER, (LIST)->next,             \
                                  struct ovs_list);                            \
          CONDITION_MULTIVAR_SAFE_LONG(VAR, NEXT, MEMBER,                      \
@@ -110,6 +111,31 @@ static inline bool ovs_list_is_short(const struct ovs_list *);
                                       ITER_VAR(NEXT) != (LIST));              \
          UPDATE_MULTIVAR_SAFE_LONG(VAR, NEXT))
 
+/* SHORT version of SAFE iterators. */
+#define LIST_FOR_EACH_REVERSE_SAFE_SHORT(VAR, MEMBER, LIST)                   \
+    for (INIT_MULTIVAR_SAFE_SHORT(VAR, MEMBER, (LIST)->prev, struct ovs_list);\
+         CONDITION_MULTIVAR_SAFE_SHORT(VAR, MEMBER,                           \
+                                       ITER_VAR(VAR) != (LIST),               \
+                                 ITER_NEXT_VAR(VAR) = ITER_VAR(VAR)->prev);   \
+         UPDATE_MULTIVAR_SAFE_SHORT(VAR))
+
+#define LIST_FOR_EACH_SAFE_SHORT(VAR, MEMBER, LIST)                           \
+    for (INIT_MULTIVAR_SAFE_SHORT(VAR, MEMBER, (LIST)->next, struct ovs_list);\
+         CONDITION_MULTIVAR_SAFE_SHORT(VAR, MEMBER,                           \
+                                       ITER_VAR(VAR) != (LIST),               \
+                                 ITER_NEXT_VAR(VAR) = ITER_VAR(VAR)->next);   \
+         UPDATE_MULTIVAR_SAFE_SHORT(VAR))
+
+#define LIST_FOR_EACH_SAFE(...)                      \
+    OVERLOAD_SAFE_MACRO(LIST_FOR_EACH_SAFE_LONG,     \
+                        LIST_FOR_EACH_SAFE_SHORT,    \
+                        4, __VA_ARGS__)
+
+#define LIST_FOR_EACH_REVERSE_SAFE(...)                        \
+    OVERLOAD_SAFE_MACRO(LIST_FOR_EACH_REVERSE_SAFE_LONG,       \
+                        LIST_FOR_EACH_REVERSE_SAFE_SHORT,      \
+                        4, __VA_ARGS__)
+
 #define LIST_FOR_EACH_POP(ITER, MEMBER, LIST)                                 \
     while (!ovs_list_is_empty(LIST) ?                                         \
            (INIT_CONTAINER(ITER, ovs_list_pop_front(LIST), MEMBER), 1) :      \

lib/conntrack.c

@@ -1526,14 +1526,14 @@ set_label(struct dp_packet *pkt, struct conn *conn,
 static long long
 ct_sweep(struct conntrack *ct, long long now, size_t limit)
 {
-    struct conn *conn, *next;
+    struct conn *conn;
     long long min_expiration = LLONG_MAX;
     size_t count = 0;
 
     ovs_mutex_lock(&ct->ct_lock);
 
     for (unsigned i = 0; i < N_CT_TM; i++) {
-        LIST_FOR_EACH_SAFE (conn, next, exp_node, &ct->exp_lists[i]) {
+        LIST_FOR_EACH_SAFE (conn, exp_node, &ct->exp_lists[i]) {
             ovs_mutex_lock(&conn->lock);
             if (now < conn->expiration || count >= limit) {
                 min_expiration = MIN(min_expiration, conn->expiration);

lib/fat-rwlock.c

@@ -97,14 +97,14 @@ fat_rwlock_init(struct fat_rwlock *rwlock)
 void
 fat_rwlock_destroy(struct fat_rwlock *rwlock)
 {
-    struct fat_rwlock_slot *slot, *next;
+    struct fat_rwlock_slot *slot;
 
     /* Order is important here.  By destroying the thread-specific data first,
      * before we destroy the slots, we ensure that the thread-specific
      * data destructor can't race with our loop below. */
     ovsthread_key_delete(rwlock->key);
 
-    LIST_FOR_EACH_SAFE (slot, next, list_node, &rwlock->threads) {
+    LIST_FOR_EACH_SAFE (slot, list_node, &rwlock->threads) {
         free_slot(slot);
     }
     ovs_mutex_destroy(&rwlock->mutex);

lib/id-fpool.c

@@ -166,11 +166,10 @@ void
 id_fpool_destroy(struct id_fpool *pool)
 {
     struct id_slab *slab;
-    struct id_slab *next;
     size_t i;
 
     id_fpool_lock(&pool->pool_lock);
-    LIST_FOR_EACH_SAFE (slab, next, node, &pool->free_slabs) {
+    LIST_FOR_EACH_SAFE (slab, node, &pool->free_slabs) {
         free(slab);
     }
     ovs_list_poison(&pool->free_slabs);

lib/ipf.c

@@ -1058,9 +1058,9 @@ ipf_send_completed_frags(struct ipf *ipf, struct dp_packet_batch *pb,
     }
 
     ovs_mutex_lock(&ipf->ipf_lock);
-    struct ipf_list *ipf_list, *next;
+    struct ipf_list *ipf_list;
 
-    LIST_FOR_EACH_SAFE (ipf_list, next, list_node, &ipf->frag_complete_list) {
+    LIST_FOR_EACH_SAFE (ipf_list, list_node, &ipf->frag_complete_list) {
         if (ipf_send_frags_in_list(ipf, ipf_list, pb, IPF_FRAG_COMPLETED_LIST,
                                    v6, now)) {
             ipf_completed_list_clean(&ipf->frag_lists, ipf_list);
@@ -1090,10 +1090,10 @@ ipf_send_expired_frags(struct ipf *ipf, struct dp_packet_batch *pb,
     }
 
     ovs_mutex_lock(&ipf->ipf_lock);
-    struct ipf_list *ipf_list, *next;
+    struct ipf_list *ipf_list;
     size_t lists_removed = 0;
 
-    LIST_FOR_EACH_SAFE (ipf_list, next, list_node, &ipf->frag_exp_list) {
+    LIST_FOR_EACH_SAFE (ipf_list, list_node, &ipf->frag_exp_list) {
         if (now <= ipf_list->expiration ||
             lists_removed >= IPF_FRAG_LIST_MAX_EXPIRED) {
             break;
@@ -1121,9 +1121,9 @@ ipf_execute_reass_pkts(struct ipf *ipf, struct dp_packet_batch *pb)
     }
 
     ovs_mutex_lock(&ipf->ipf_lock);
-    struct reassembled_pkt *rp, *next;
+    struct reassembled_pkt *rp;
 
-    LIST_FOR_EACH_SAFE (rp, next, rp_list_node, &ipf->reassembled_pkt_list) {
+    LIST_FOR_EACH_SAFE (rp, rp_list_node, &ipf->reassembled_pkt_list) {
         if (!rp->list->reass_execute_ctx &&
             ipf_dp_packet_batch_add(pb, rp->pkt, false)) {
             rp->list->reass_execute_ctx = rp->pkt;
@@ -1144,9 +1144,9 @@ ipf_post_execute_reass_pkts(struct ipf *ipf,
     }
 
     ovs_mutex_lock(&ipf->ipf_lock);
-    struct reassembled_pkt *rp, *next;
+    struct reassembled_pkt *rp;
 
-    LIST_FOR_EACH_SAFE (rp, next, rp_list_node, &ipf->reassembled_pkt_list) {
+    LIST_FOR_EACH_SAFE (rp, rp_list_node, &ipf->reassembled_pkt_list) {
         const size_t pb_cnt = dp_packet_batch_size(pb);
         int pb_idx;
         struct dp_packet *pkt;
@@ -1271,15 +1271,15 @@ ipf_clean_thread_main(void *f)
 
             ovs_mutex_lock(&ipf->ipf_lock);
 
-            struct ipf_list *ipf_list, *next;
-            LIST_FOR_EACH_SAFE (ipf_list, next, list_node,
+            struct ipf_list *ipf_list;
+            LIST_FOR_EACH_SAFE (ipf_list, list_node,
                                 &ipf->frag_exp_list) {
                 if (ipf_purge_list_check(ipf, ipf_list, now)) {
                     ipf_expiry_list_clean(&ipf->frag_lists, ipf_list);
                 }
             }
 
-            LIST_FOR_EACH_SAFE (ipf_list, next, list_node,
+            LIST_FOR_EACH_SAFE (ipf_list, list_node,
                                 &ipf->frag_complete_list) {
                 if (ipf_purge_list_check(ipf, ipf_list, now)) {
                     ipf_completed_list_clean(&ipf->frag_lists, ipf_list);

lib/lldp/lldpd-structs.c

@@ -64,11 +64,11 @@ lldpd_remote_cleanup(struct lldpd_hardware *hw,
                                    struct lldpd_port *),
                      bool all)
 {
-    struct lldpd_port *port, *port_next;
+    struct lldpd_port *port;
     time_t now = time_now();
 
     VLOG_DBG("cleanup remote port on %s", hw->h_ifname);
-    LIST_FOR_EACH_SAFE (port, port_next, p_entries, &hw->h_rports) {
+    LIST_FOR_EACH_SAFE (port, p_entries, &hw->h_rports) {
         bool del = all;
         if (!all && expire &&
             (now >= port->p_lastupdate + port->p_chassis->c_ttl)) {
@@ -99,11 +99,10 @@ static void
 lldpd_aa_maps_cleanup(struct lldpd_port *port)
 {
     struct lldpd_aa_isid_vlan_maps_tlv *isid_vlan_map = NULL;
-    struct lldpd_aa_isid_vlan_maps_tlv *isid_vlan_map_next = NULL;
 
     if (!ovs_list_is_empty(&port->p_isid_vlan_maps)) {
 
-        LIST_FOR_EACH_SAFE (isid_vlan_map, isid_vlan_map_next, m_entries,
+        LIST_FOR_EACH_SAFE (isid_vlan_map, m_entries,
                             &port->p_isid_vlan_maps) {
 
             ovs_list_remove(&isid_vlan_map->m_entries);

lib/lldp/lldpd.c

@@ -134,12 +134,12 @@ lldpd_hardware_cleanup(struct lldpd *cfg, struct lldpd_hardware *hardware)
 void
 lldpd_cleanup(struct lldpd *cfg)
 {
-    struct lldpd_hardware *hw, *hw_next;
-    struct lldpd_chassis *chassis, *chassis_next;
+    struct lldpd_hardware *hw;
+    struct lldpd_chassis *chassis;
 
     VLOG_DBG("cleanup all ports");
 
-    LIST_FOR_EACH_SAFE (hw, hw_next, h_entries, &cfg->g_hardware) {
+    LIST_FOR_EACH_SAFE (hw, h_entries, &cfg->g_hardware) {
         if (!hw->h_flags) {
             ovs_list_remove(&hw->h_entries);
             lldpd_remote_cleanup(hw, NULL, true);
@@ -151,7 +151,7 @@ lldpd_cleanup(struct lldpd *cfg)
 
     VLOG_DBG("cleanup all chassis");
 
-    LIST_FOR_EACH_SAFE (chassis, chassis_next, list, &cfg->g_chassis) {
+    LIST_FOR_EACH_SAFE (chassis, list, &cfg->g_chassis) {
         if (chassis->c_refcount == 0) {
             ovs_list_remove(&chassis->list);
             lldpd_chassis_cleanup(chassis, 1);

lib/mcast-snooping.c

@@ -356,11 +356,11 @@ mcast_snooping_prune_expired(struct mcast_snooping *ms,
     OVS_REQ_WRLOCK(ms->rwlock)
 {
     int expired;
-    struct mcast_group_bundle *b, *next_b;
+    struct mcast_group_bundle *b;
     time_t timenow = time_now();
 
     expired = 0;
-    LIST_FOR_EACH_SAFE (b, next_b, bundle_node, &grp->bundle_lru) {
+    LIST_FOR_EACH_SAFE (b, bundle_node, &grp->bundle_lru) {
         /* This list is sorted on expiration time. */
         if (b->expires > timenow) {
             break;
@@ -946,15 +946,15 @@ mcast_snooping_wait(struct mcast_snooping *ms)
 void
 mcast_snooping_flush_bundle(struct mcast_snooping *ms, void *port)
 {
-    struct mcast_group *g, *next_g;
-    struct mcast_mrouter_bundle *m, *next_m;
+    struct mcast_group *g;
+    struct mcast_mrouter_bundle *m;
 
     if (!mcast_snooping_enabled(ms)) {
         return;
     }
 
     ovs_rwlock_wrlock(&ms->rwlock);
-    LIST_FOR_EACH_SAFE (g, next_g, group_node, &ms->group_lru) {
+    LIST_FOR_EACH_SAFE (g, group_node, &ms->group_lru) {
         if (mcast_group_delete_bundle(ms, g, port)) {
             ms->need_revalidate = true;
 
@@ -964,7 +964,7 @@ mcast_snooping_flush_bundle(struct mcast_snooping *ms, void *port)
         }
     }
 
-    LIST_FOR_EACH_SAFE (m, next_m, mrouter_node, &ms->mrouter_lru) {
+    LIST_FOR_EACH_SAFE (m, mrouter_node, &ms->mrouter_lru) {
         if (m->port == port) {
             mcast_snooping_flush_mrouter(m);
             ms->need_revalidate = true;

lib/netdev-afxdp.c

@@ -235,11 +235,11 @@ netdev_afxdp_cleanup_unused_pool(struct unused_pool *pool)
 static void
 netdev_afxdp_sweep_unused_pools(void *aux OVS_UNUSED)
 {
-    struct unused_pool *pool, *next;
+    struct unused_pool *pool;
     unsigned int count;
 
     ovs_mutex_lock(&unused_pools_mutex);
-    LIST_FOR_EACH_SAFE (pool, next, list_node, &unused_pools) {
+    LIST_FOR_EACH_SAFE (pool, list_node, &unused_pools) {
 
         count = umem_pool_count(&pool->umem_info->mpool);
         ovs_assert(count + pool->lost_in_rings <= NUM_FRAMES);

lib/netdev-dpdk.c

@@ -623,9 +623,9 @@ dpdk_mp_full(const struct rte_mempool *mp) OVS_REQUIRES(dpdk_mp_mutex)
 static void
 dpdk_mp_sweep(void) OVS_REQUIRES(dpdk_mp_mutex)
 {
-    struct dpdk_mp *dmp, *next;
+    struct dpdk_mp *dmp;
 
-    LIST_FOR_EACH_SAFE (dmp, next, list_node, &dpdk_mp_list) {
+    LIST_FOR_EACH_SAFE (dmp, list_node, &dpdk_mp_list) {
         if (!dmp->refcount && dpdk_mp_full(dmp->mp)) {
             VLOG_DBG("Freeing mempool \"%s\"", dmp->mp->name);
             ovs_list_remove(&dmp->list_node);

lib/ofp-msgs.c

@@ -1290,8 +1290,8 @@ ofpmp_assembler_execute(struct hmap *assembler, struct ofpbuf *msg,
      * on either side by parts with 0-byte bodies.  We remove the 0-byte
      * ones here to simplify processing later.
      */
-    struct ofpbuf *b, *next;
-    LIST_FOR_EACH_SAFE (b, next, list_node, out) {
+    struct ofpbuf *b;
+    LIST_FOR_EACH_SAFE (b, list_node, out) {
         if (b->size <= min_len && !ovs_list_is_short(out)) {
             ovs_list_remove(&b->list_node);
             ofpbuf_delete(b);

lib/ovs-lldp.c

@@ -559,9 +559,9 @@ aa_mapping_unregister_mapping(struct lldp *lldp,
                               struct lldpd_hardware *hw,
                               struct aa_mapping_internal *m)
 {
-    struct lldpd_aa_isid_vlan_maps_tlv *lm, *lm_next;
+    struct lldpd_aa_isid_vlan_maps_tlv *lm;
 
-    LIST_FOR_EACH_SAFE (lm, lm_next, m_entries,
+    LIST_FOR_EACH_SAFE (lm, m_entries,
                         &hw->h_lport.p_isid_vlan_maps) {
         uint32_t isid = lm->isid_vlan_data.isid;
 
@@ -953,8 +953,8 @@ lldp_ref(const struct lldp *lldp_)
 void
 lldp_destroy_dummy(struct lldp *lldp)
 {
-    struct lldpd_hardware *hw, *hw_next;
-    struct lldpd_chassis *chassis, *chassis_next;
+    struct lldpd_hardware *hw;
+    struct lldpd_chassis *chassis;
     struct lldpd *cfg;
 
     if (!lldp) {
@@ -963,13 +963,13 @@ lldp_destroy_dummy(struct lldp *lldp)
 
     cfg = lldp->lldpd;
 
-    LIST_FOR_EACH_SAFE (hw, hw_next, h_entries, &cfg->g_hardware) {
+    LIST_FOR_EACH_SAFE (hw, h_entries, &cfg->g_hardware) {
         ovs_list_remove(&hw->h_entries);
         free(hw->h_lport.p_lastframe);
         free(hw);
     }
 
-    LIST_FOR_EACH_SAFE (chassis, chassis_next, list, &cfg->g_chassis) {
+    LIST_FOR_EACH_SAFE (chassis, list, &cfg->g_chassis) {
         ovs_list_remove(&chassis->list);
         free(chassis);
     }