On Mon, Oct 09, 2017 at 06:56:21PM -0700, Greg Rose wrote:
> On 10/09/2017 04:34 PM, Ben Pfaff wrote:
> >On Mon, Oct 09, 2017 at 03:59:37PM -0700, Greg Rose wrote:
> >> Fix macro for case where build is done with NDEBUG
> >>
> >> Signed-off-by: Greg Rose <gvrose8...@gmail.com>
> >> ---
> >> include/openvswitch/util.h | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/include/openvswitch/util.h b/include/openvswitch/util.h
> >> index abebf96..72299e7 100644
> >> --- a/include/openvswitch/util.h
> >> +++ b/include/openvswitch/util.h
> >> @@ -52,7 +52,7 @@ const char *ovs_get_program_version(void);
> >> ovs_assert_failure(OVS_SOURCE_LOCATOR, __func__, #CONDITION);
> >> \
> >> }
> >> #else
> >> -#define ovs_assert(CONDITION) ((void) (CONDITION))
> >> +#define ovs_assert(CONDITION) ((void) (CONDITION));
> >> #endif
> >
> >We normally invoke ovs_assert() like a function call, followed by a
> >semicolon. If there's a missing ; somewhere in a user of ovs_assert(),
> >we should fix that.
> >
> >The other case does look weird to me, because in normal usage it will
> >expand to something like:
> > if (...) {
> > ...
> > };
> >so it might be a good idea to make it a single expression, maybe like
> >this:
> >
> >diff --git a/include/openvswitch/util.h b/include/openvswitch/util.h
> >index abebf96fa3ac..84d7e07ff370 100644
> >--- a/include/openvswitch/util.h
> >+++ b/include/openvswitch/util.h
> >@@ -48,9 +48,9 @@ const char *ovs_get_program_version(void);
> > * log. */
> > #ifndef NDEBUG
> > #define ovs_assert(CONDITION) \
> >- if (!OVS_LIKELY(CONDITION)) { \
> >- ovs_assert_failure(OVS_SOURCE_LOCATOR, __func__, #CONDITION);
> >\
> >- }
> >+ (OVS_LIKELY(CONDITION) \
> >+ ? (void) 0 \
> >+ : ovs_assert_failure(OVS_SOURCE_LOCATOR, __func__, #CONDITION))
> > #else
> > #define ovs_assert(CONDITION) ((void) (CONDITION))
> > #endif
> >
> Yep, that makes more sense to me... I was just trying to do something else
> and ran into this
> so didn't think on it much.
>
> Looks good to me!
Thanks! I applied this to master.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
