On 2/15/22 13:58, Adrian Moreno wrote: > > > On 2/15/22 13:54, Ilya Maximets wrote: >> 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. >> > > Not sure if that'll work for all cases. For instance, I don't think declaring > the iterator as void* will work with data structures that use a function to > calculate the next iterator value (e.g: hmap_next). >
I guess, you can explicitly convert in such cases while calling the function. But if you can use explicit types everywhere without massive code duplication, that might be better, I think. _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
