[ovs-dev] [PATCH] include/openvswitch/utils.h:

Ben Pfaff blp at ovn.org
Mon Oct 9 23:34:56 UTC 2017


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 <gvrose8192 at 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


More information about the dev mailing list