On 2/15/22 13:49, Adrian Moreno wrote: > > > 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. >
Try OVS_TYPEOF macro. It simply converts to void * in that case though. _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
