On 2/15/22 13:42, Adrian Moreno wrote:


On 2/14/22 18:30, Ilya Maximets wrote:
On 2/14/22 11:13, Adrian Moreno wrote:
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.

Signed-off-by: Adrian Moreno <amore...@redhat.com>
---
  include/openvswitch/list.h   | 32 +++++++++++++++++++++++++++++--
  lib/conntrack.c              |  4 ++--
  lib/fat-rwlock.c             |  4 ++--
  lib/id-fpool.c               |  3 +--
  lib/ipf.c                    | 22 ++++++++++-----------
  lib/lldp/lldpd-structs.c     |  7 +++----
  lib/lldp/lldpd.c             |  8 ++++----
  lib/mcast-snooping.c         | 12 ++++++------
  lib/netdev-afxdp.c           |  4 ++--
  lib/netdev-dpdk.c            |  4 ++--
  lib/ofp-msgs.c               |  4 ++--
  lib/ovs-lldp.c               | 12 ++++++------
  lib/ovsdb-idl.c              | 30 ++++++++++++++---------------
  lib/seq.c                    |  4 ++--
  lib/tnl-ports.c              | 16 ++++++++--------
  lib/unixctl.c                |  8 ++++----
  lib/vconn.c                  |  4 ++--
  ofproto/connmgr.c            |  8 ++++----
  ofproto/ofproto-dpif-ipfix.c |  4 ++--
  ofproto/ofproto-dpif-trace.c |  4 ++--
  ofproto/ofproto-dpif-xlate.c |  4 ++--
  ofproto/ofproto-dpif.c       | 24 +++++++++++------------
  ovsdb/jsonrpc-server.c       | 16 ++++++++--------
  ovsdb/monitor.c              | 24 +++++++++++------------
  ovsdb/ovsdb.c                |  4 ++--
  ovsdb/raft.c                 | 15 +++++++--------
  ovsdb/transaction-forward.c  |  6 +++---
  ovsdb/transaction.c          | 28 +++++++++++++--------------
  ovsdb/trigger.c              |  4 ++--
  tests/test-list.c            | 37 ++++++++++++++++++++++++++++++++++++
  utilities/ovs-ofctl.c        |  4 ++--
  utilities/ovs-vsctl.c        |  8 ++++----
  vswitchd/bridge.c            | 16 ++++++++--------
  vtep/vtep-ctl.c              | 12 ++++++------
  34 files changed, 228 insertions(+), 168 deletions(-)

diff --git a/include/openvswitch/list.h b/include/openvswitch/list.h
index 997afc0e4..c6941e896 100644
--- a/include/openvswitch/list.h
+++ b/include/openvswitch/list.h
@@ -93,7 +93,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) = 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);            \            CONDITION_MULTIVAR_SAFE_LONG(VAR, PREV, MEMBER,                      \                                         ITER_VAR(VAR) != (LIST),                \ @@ -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);            \            CONDITION_MULTIVAR_SAFE_LONG(VAR, NEXT, MEMBER,                      \                                         ITER_VAR(VAR) != (LIST),                \ @@ -109,6 +110,33 @@ 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);                 \
+         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);                 \
+         CONDITION_MULTIVAR_SAFE_SHORT(VAR, MEMBER,                           \
+                                       ITER_VAR(VAR) != (LIST),               \
+                                 ITER_NEXT_VAR(VAR) = ITER_VAR(VAR)->next);   \
+         UPDATE_MULTIVAR_SAFE_SHORT(VAR))
+
+/* Select the right SAFE macro depending on the number of arguments .*/
+#define LIST_GET_SAFE_MACRO(_1, _2, _3, _4, NAME, ...) NAME
+#define LIST_FOR_EACH_SAFE(...)                                               \
+    LIST_GET_SAFE_MACRO(__VA_ARGS__,                                          \
+                   LIST_FOR_EACH_SAFE_LONG,                                   \
+                   LIST_FOR_EACH_SAFE_SHORT)(__VA_ARGS__)

Hey, Adrian.

I should have said that upfront, but there is a reason I pointed to the
stackoverflow answer related to MSVC.  It appears that MSVC preprocessor
doesn't handle __VA_ARGS__ expansion in the same way as GCC and others
do, unless experimental preprocessor is enabled.  So, it seem to require
special handling:
https://developercommunity.visualstudio.com/t/-va-args-seems-to-be-trated-as-a-single-parameter/460154

Did you check this patch set on Windows, e.g. with AppVeyor?
In any case, we're requiring Visual Studio 2013+ to build OVS and this
preprocessor thing doesn't seem to be fixed in that version.

Best regards, Ilya Maximets.


Hi Ilya,

Thanks for pointing it out. I had not tried on Windows. In fact, there's a more serious problem with MVSC: it lacks typeof(). This whole approach is based on being able to declare a variable inside the loop of typeof(&VAR->MEMBER). So I wonder if it's even possible to do this on Windows without having to maintain two versions of all the loop macros.

Thanks.


I'll look into hardcoding the type, see if that's enough to please MVSC.

--
Adrián Moreno

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to