Skip to contentSkip to conversation

Replace MSG_TEST() macro for static inline msg_test()

PATCH In review Steffan Karger 2 participants 3 messages Mar 27, 2016 – Apr 4, 2016

lore
Share & tools
lore

Steffan Karger

author
#0 lore raw

PATCH 1/1 Replace MSG_TEST() macro for static inline msg_test()

Using a static inline function instead of a macro has the advantages that
(1) 'flags' is not evaluated twice and (2) coverity will stop complaining that 'Macro compares unsigned to 0 (NO_EFFECT)' each time we use flags with loglevel 0 (e.g. M_FATAL or M_WARN).

This has a performance impact when compiler optimizations are fully disabled ('-O0'), but should otherwise be as fast as using a macro.

Signed-off-by Steffan Karger <[email protected]...>
---
 src/openvpn/error.c  |  2 +-
 src/openvpn/error.h  | 17 +++++++++++------
 src/openvpn/plugin.c |  2 +-
 3 files changed, 13 insertions(+), 8 deletions(-)
src/openvpn/error.c +1 -1
index cfd5a41..bb0ab5b 100644
@@ -228,7 +228,7 @@ void x_msg_va (const unsigned int flags, const char *format, va_list arglist)
 
 #ifndef HAVE_VARARG_MACROS
   /* the macro has checked this otherwise */
-  if (!MSG_TEST (flags))
+  if (!msg_test (flags))
     return;
 #endif
src/openvpn/error.h +11 -6
index dd5ccf7..76515d6 100644
@@ -135,26 +135,31 @@ extern int x_msg_line_num;
  * msg() as a macro for optimization win.
  */
 
-bool dont_mute (unsigned int flags); /* check muting filter */
+/** Check muting filter */
+bool dont_mute (unsigned int flags);
 
-#define MSG_TEST(flags) (unlikely((((unsigned int)flags) & M_DEBUG_LEVEL) <= x_debug_level) && dont_mute (flags))
+/** Return true if flags represent an enabled, not muted log level */
+static inline bool msg_test (unsigned int flags)
+{
+  return ((flags & M_DEBUG_LEVEL) <= x_debug_level) && dont_mute (flags);
+}
 
 /* Macro to ensure (and teach static analysis tools) we exit on fatal errors */
 #define EXIT_FATAL(flags) do { if ((flags) & M_FATAL) _exit(1); } while (false)
 
 #if defined(HAVE_CPP_VARARG_MACRO_ISO) && !defined(__LCLINT__)
 # define HAVE_VARARG_MACROS
-# define msg(flags, ...) do { if (MSG_TEST(flags)) x_msg((flags), __VA_ARGS__); EXIT_FATAL(flags); } while (false)
+# define msg(flags, ...) do { if (msg_test(flags)) x_msg((flags), __VA_ARGS__); EXIT_FATAL(flags); } while (false)
 # ifdef ENABLE_DEBUG
-#  define dmsg(flags, ...) do { if (MSG_TEST(flags)) x_msg((flags), __VA_ARGS__); EXIT_FATAL(flags); } while (false)
+#  define dmsg(flags, ...) do { if (msg_test(flags)) x_msg((flags), __VA_ARGS__); EXIT_FATAL(flags); } while (false)
 # else
 #  define dmsg(flags, ...)
 # endif
 #elif defined(HAVE_CPP_VARARG_MACRO_GCC) && !defined(__LCLINT__)
 # define HAVE_VARARG_MACROS
-# define msg(flags, args...) do { if (MSG_TEST(flags)) x_msg((flags), args); EXIT_FATAL(flags); } while (false)
+# define msg(flags, args...) do { if (msg_test(flags)) x_msg((flags), args); EXIT_FATAL(flags); } while (false)
 # ifdef ENABLE_DEBUG
-#  define dmsg(flags, args...) do { if (MSG_TEST(flags)) x_msg((flags), args); EXIT_FATAL(flags); } while (false)
+#  define dmsg(flags, args...) do { if (msg_test(flags)) x_msg((flags), args); EXIT_FATAL(flags); } while (false)
 # else
 #  define dmsg(flags, args...)
 # endif
src/openvpn/plugin.c +1 -1
index 9be0b0c..542e5b1 100644
@@ -319,7 +319,7 @@ plugin_vlog (openvpn_plugin_log_flags_t flags, const char *name, const char *for
   if (flags & PLOG_NOMUTE)
     msg_flags |= M_NOMUTE;
 
-  if (MSG_TEST (msg_flags))
+  if (msg_test (msg_flags))
     {
       struct gc_arena gc;
       char* msg_fmt;
Signature · 1 line
2.5.0
#1 lore raw

ACK. For the reasons you named :-)

Your patch has been applied to the master branch.

commit bbde0a766c69f573746461415c6f5cd289272fff

Author: Steffan Karger
Date:   Sun Mar 27 16:18:16 2016 +0200
     Replace MSG_TEST() macro for static inline msg_test()

     Signed-off-by: Steffan Karger <[email protected]...>
     Acked-by: Gert Doering <[email protected]...>
     Message-Id: <[email protected]...>
     URL: http://article.gmane.org/gmane.network.openvpn.devel/11368
     Signed-off-by: Gert Doering <[email protected]...>

--
kind regards,

Gert Doering

#2 lore raw

Hi,

Quoted 3 lines · Gert Doering

ACK. For the reasons you named :-)

Your patch has been applied to the master branch.

And also to release/2.3 (after feedback on IRC) - since that is the current branch used for coverity.

commit 68a8c3a26c25ab478611a7fee209a9ecc65c59ce (release/2.3)

gert

Signature · 4 lines
USENET is *not* the non-clickable part of WWW!
                                                           //www.muc.de/~gert/
Gert Doering - Munich, Germany                             [email protected]...
fax: +49-89-35655025                        [email protected]...