| Message ID | 20220309161023.3347758-4-amorenoz@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Fix undefined behavior in loop macros | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/intel-ovs-compilation | success | test: success |
On 9 Mar 2022, at 17:10, Adrian Moreno wrote: > Having both LONG and SHORT versions of the SAFE macros with different > names is not very convenient. Add helpers that facilitate overloading > such macros using a single name. > > In order to work around a known issue in MSVC [1], an indirection layer > has to be introduced. > > [1] > https://developercommunity.visualstudio.com/t/-va-args-seems-to-be-trated-as-a-single-parameter/460154 > > Acked-by: Dumitru Ceara <dceara@redhat.com> > Signed-off-by: Adrian Moreno <amorenoz@redhat.com> > --- This looks good to me! Acked-by: Eelco Chaudron <echaudro@redhat.com> Small nit, do we need to update the date in the copyright notice? I keep forgetting if we do or do not. But I guess not, as I do not see it in other files. > include/openvswitch/util.h | 21 +++++++++++++++++++++ > 1 file changed, 21 insertions(+) > > diff --git a/include/openvswitch/util.h b/include/openvswitch/util.h > index 157f8cecd..8184dd3d3 100644 > --- a/include/openvswitch/util.h > +++ b/include/openvswitch/util.h > @@ -268,6 +268,27 @@ OVS_NO_RETURN void ovs_assert_failure(const char *, const char *, const char *); > #define UPDATE_MULTIVAR_SAFE_LONG(VAR, NEXT_VAR) \ > UPDATE_MULTIVAR(VAR, ITER_VAR(NEXT_VAR)) > > +/* Helpers to allow overloading the *_SAFE iterator macros and select either > + * the LONG or the SHORT version depending on the number of arguments. > + */ > +#define GET_SAFE_MACRO2(_1, _2, NAME, ...) NAME > +#define GET_SAFE_MACRO3(_1, _2, _3, NAME, ...) NAME > +#define GET_SAFE_MACRO4(_1, _2, _3, _4, NAME, ...) NAME > +#define GET_SAFE_MACRO5(_1, _2, _3, _4, _5, NAME, ...) NAME > +#define GET_SAFE_MACRO6(_1, _2, _3, _4, _5, _6, NAME, ...) NAME > +#define GET_SAFE_MACRO(MAX_ARGS) GET_SAFE_MACRO ## MAX_ARGS > + > +/* MSVC treats __VA_ARGS__ as a simple token in argument lists. Introduce > + * a level of indirection to work around that. */ > +#define EXPAND_MACRO(name, args) name args > + > +/* Overload the LONG and the SHORT version of the macros. MAX_ARGS is the > + * maximum number of arguments (i.e: the number of arguments of the LONG > + * version). */ > +#define OVERLOAD_SAFE_MACRO(LONG, SHORT, MAX_ARGS, ...) \ > + EXPAND_MACRO(GET_SAFE_MACRO(MAX_ARGS), \ > + (__VA_ARGS__, LONG, SHORT))(__VA_ARGS__) > + > /* Returns the number of elements in ARRAY. */ > #define ARRAY_SIZE(ARRAY) __ARRAY_SIZE(ARRAY) > > -- > 2.34.1 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
On 3/18/22 14:19, Eelco Chaudron wrote: > > > On 9 Mar 2022, at 17:10, Adrian Moreno wrote: > >> Having both LONG and SHORT versions of the SAFE macros with different >> names is not very convenient. Add helpers that facilitate overloading >> such macros using a single name. >> >> In order to work around a known issue in MSVC [1], an indirection layer >> has to be introduced. >> >> [1] >> https://developercommunity.visualstudio.com/t/-va-args-seems-to-be-trated-as-a-single-parameter/460154 >> >> Acked-by: Dumitru Ceara <dceara@redhat.com> >> Signed-off-by: Adrian Moreno <amorenoz@redhat.com> >> --- > > This looks good to me! > > Acked-by: Eelco Chaudron <echaudro@redhat.com> > Thanks for the review Eelco. > > Small nit, do we need to update the date in the copyright notice? I keep forgetting if we do or do not. But I guess not, as I do not see it in other files. > I honestly have no idea. Maybe Ilya can help here? >> include/openvswitch/util.h | 21 +++++++++++++++++++++ >> 1 file changed, 21 insertions(+) >> >> diff --git a/include/openvswitch/util.h b/include/openvswitch/util.h >> index 157f8cecd..8184dd3d3 100644 >> --- a/include/openvswitch/util.h >> +++ b/include/openvswitch/util.h >> @@ -268,6 +268,27 @@ OVS_NO_RETURN void ovs_assert_failure(const char *, const char *, const char *); >> #define UPDATE_MULTIVAR_SAFE_LONG(VAR, NEXT_VAR) \ >> UPDATE_MULTIVAR(VAR, ITER_VAR(NEXT_VAR)) >> >> +/* Helpers to allow overloading the *_SAFE iterator macros and select either >> + * the LONG or the SHORT version depending on the number of arguments. >> + */ >> +#define GET_SAFE_MACRO2(_1, _2, NAME, ...) NAME >> +#define GET_SAFE_MACRO3(_1, _2, _3, NAME, ...) NAME >> +#define GET_SAFE_MACRO4(_1, _2, _3, _4, NAME, ...) NAME >> +#define GET_SAFE_MACRO5(_1, _2, _3, _4, _5, NAME, ...) NAME >> +#define GET_SAFE_MACRO6(_1, _2, _3, _4, _5, _6, NAME, ...) NAME >> +#define GET_SAFE_MACRO(MAX_ARGS) GET_SAFE_MACRO ## MAX_ARGS >> + >> +/* MSVC treats __VA_ARGS__ as a simple token in argument lists. Introduce >> + * a level of indirection to work around that. */ >> +#define EXPAND_MACRO(name, args) name args >> + >> +/* Overload the LONG and the SHORT version of the macros. MAX_ARGS is the >> + * maximum number of arguments (i.e: the number of arguments of the LONG >> + * version). */ >> +#define OVERLOAD_SAFE_MACRO(LONG, SHORT, MAX_ARGS, ...) \ >> + EXPAND_MACRO(GET_SAFE_MACRO(MAX_ARGS), \ >> + (__VA_ARGS__, LONG, SHORT))(__VA_ARGS__) >> + >> /* Returns the number of elements in ARRAY. */ >> #define ARRAY_SIZE(ARRAY) __ARRAY_SIZE(ARRAY) >> >> -- >> 2.34.1 >> >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
diff --git a/include/openvswitch/util.h b/include/openvswitch/util.h index 157f8cecd..8184dd3d3 100644 --- a/include/openvswitch/util.h +++ b/include/openvswitch/util.h @@ -268,6 +268,27 @@ OVS_NO_RETURN void ovs_assert_failure(const char *, const char *, const char *); #define UPDATE_MULTIVAR_SAFE_LONG(VAR, NEXT_VAR) \ UPDATE_MULTIVAR(VAR, ITER_VAR(NEXT_VAR)) +/* Helpers to allow overloading the *_SAFE iterator macros and select either + * the LONG or the SHORT version depending on the number of arguments. + */ +#define GET_SAFE_MACRO2(_1, _2, NAME, ...) NAME +#define GET_SAFE_MACRO3(_1, _2, _3, NAME, ...) NAME +#define GET_SAFE_MACRO4(_1, _2, _3, _4, NAME, ...) NAME +#define GET_SAFE_MACRO5(_1, _2, _3, _4, _5, NAME, ...) NAME +#define GET_SAFE_MACRO6(_1, _2, _3, _4, _5, _6, NAME, ...) NAME +#define GET_SAFE_MACRO(MAX_ARGS) GET_SAFE_MACRO ## MAX_ARGS + +/* MSVC treats __VA_ARGS__ as a simple token in argument lists. Introduce + * a level of indirection to work around that. */ +#define EXPAND_MACRO(name, args) name args + +/* Overload the LONG and the SHORT version of the macros. MAX_ARGS is the + * maximum number of arguments (i.e: the number of arguments of the LONG + * version). */ +#define OVERLOAD_SAFE_MACRO(LONG, SHORT, MAX_ARGS, ...) \ + EXPAND_MACRO(GET_SAFE_MACRO(MAX_ARGS), \ + (__VA_ARGS__, LONG, SHORT))(__VA_ARGS__) + /* Returns the number of elements in ARRAY. */ #define ARRAY_SIZE(ARRAY) __ARRAY_SIZE(ARRAY)