[network-manager-openvpn/th/vpn-plugin-debug-bgo766816: 6/6] service: cleanup conditional logging macros

[Thomas Haller <thaller src gnome org>]

commit 011afc1bb93916f728b17fa90ebdb81f7cca77c2
Author: Thomas Haller <thaller redhat com>
Date:   Tue May 24 16:34:29 2016 +0200

    service: cleanup conditional logging macros
    
    Previously, the logging macros _LOGI() and _LOGW() would
    unconditionally log, without considering the logging level.
    Only _LOGD() would consider the logging level.
    
    Now that we can configure logging levels, that is strange. Why would we
    have a logging macro _LOGI() logging info-messages unless info
    level enabled?
    
    Previously, before this branch, logging was like this:
      - some logging statements _LOGW(), _LOGI() were always logged by the
        service.
      - some logging statements _LOGd() were only logged when called with
        --debug.
      - nm-openvpn-service-helper would always log messages via g_log().
      - openvpn was called with "--verb 10" in debug mode, otherwise without
        verb options (which equals "--verb 1").
    Especially, NetworkManager would spawn the plugin without --debug
    option.
    
    Now, newer NetworkManager spawns the plugin with NM_VPN_LOG_LEVEL.
    In this case, the logging level set by NetworkManager is authorative
    for all statements. And the "--verb" option for openvpn directly
    corresponds.
    Most notably, NetworkManager quite likely calls the plugin with NM_VPN_LOG_LEVEL=0,
    in which case all _LOGX() macros are disabled, and openvpn is called
    with "--verb 1".
    So, in such a situation, the plugin now logs almost nothing.
    
    This obviously is a change in behavior, but "logging" doesn't guarantee
    stable behavior, and the new behavior makes sense. If the user wants to
    enable logging of the plugins, he should do
    
      $ nmcli general logging level KEEP domains VPN_PLUGIN:INFO

 src/nm-openvpn-service.c |   45 +++++++++++++++++++++++++++++----------------
 1 files changed, 29 insertions(+), 16 deletions(-)
---
diff --git a/src/nm-openvpn-service.c b/src/nm-openvpn-service.c
index 1f711a4..7de0d79 100644
--- a/src/nm-openvpn-service.c
+++ b/src/nm-openvpn-service.c
@@ -160,9 +160,15 @@ static ValidProperty valid_secrets[] = {
 
 /*****************************************************************************/
 
-#define _NMLOG(level, log_always, ...) \
+static int
+_log_level (void)
+{
+       return gl.log_level >= 0 ? gl.log_level : (gl.debug ? LOG_DEBUG : LOG_INFO);
+}
+
+#define _NMLOG(level, ...) \
        G_STMT_START { \
-               if ((log_always) || gl.log_level >= (level)) { \
+               if (_log_level () >= (level)) { \
                        g_print ("nm-openvpn[%ld] %-7s " _NM_UTILS_MACRO_FIRST (__VA_ARGS__) "\n", \
                                 (long) getpid (), \
                                 nm_utils_syslog_to_str (level) \
@@ -173,12 +179,12 @@ static ValidProperty valid_secrets[] = {
 static gboolean
 _LOGD_enabled (void)
 {
-       return gl.log_level >= LOG_INFO;
+       return _log_level () >= LOG_INFO;
 }
 
-#define _LOGD(...) _NMLOG(LOG_DEBUG,   FALSE, __VA_ARGS__)
-#define _LOGI(...) _NMLOG(LOG_INFO,    TRUE,  __VA_ARGS__)
-#define _LOGW(...) _NMLOG(LOG_WARNING, TRUE,  __VA_ARGS__)
+#define _LOGD(...) _NMLOG(LOG_DEBUG,   __VA_ARGS__)
+#define _LOGI(...) _NMLOG(LOG_INFO,    __VA_ARGS__)
+#define _LOGW(...) _NMLOG(LOG_WARNING, __VA_ARGS__)
 
 /*****************************************************************************/
 
@@ -1105,6 +1111,8 @@ nm_openvpn_start_openvpn_binary (NMOpenvpnPlugin *plugin,
        const char *defport, *proto_tcp;
        const char *nm_openvpn_user, *nm_openvpn_group, *nm_openvpn_chroot;
        gs_free char *bus_name = NULL;
+       int log_level_ovpn;
+       char s_buf_verb[20];
 
        /* Find openvpn */
        openvpn_binary = nm_find_openvpn ();
@@ -1423,14 +1431,21 @@ nm_openvpn_start_openvpn_binary (NMOpenvpnPlugin *plugin,
                add_openvpn_arg (args, "0");
        }
 
-       if (gl.log_level > 0) {
-               add_openvpn_arg (args, "--verb");
+       if (gl.debug || gl.log_level >= 0) {
                if (gl.log_level >= LOG_DEBUG)
-                       add_openvpn_arg (args, "10");
+                       log_level_ovpn = 10;
                else if (gl.log_level >= LOG_INFO)
-                       add_openvpn_arg (args, "5");
-               else
-                       add_openvpn_arg (args, "2");
+                       log_level_ovpn = 5;
+               else if (gl.log_level > 0)
+                       log_level_ovpn = 2;
+               else if (gl.log_level == 0)
+                       log_level_ovpn = 1;
+               else {
+                       /* only gl.debug is set. */
+                       log_level_ovpn = 10;
+               }
+               add_openvpn_arg (args, "--verb");
+               add_openvpn_arg (args, nm_sprintf_buf (s_buf_verb, "%d", log_level_ovpn));
        } else {
                /* the default level is already "--verb 1", which is fine for us. */
        }
@@ -1488,7 +1503,7 @@ nm_openvpn_start_openvpn_binary (NMOpenvpnPlugin *plugin,
        g_object_get (plugin, NM_VPN_SERVICE_PLUGIN_DBUS_SERVICE_NAME, &bus_name, NULL);
        stmp = g_strdup_printf ("%s --debug %d %ld --bus-name %s %s --",
                                NM_OPENVPN_HELPER_PATH,
-                               gl.log_level, (long) getpid(),
+                               _log_level (), (long) getpid(),
                                bus_name,
                                dev_type_is_tap ? "--tap" : "--tun");
        add_openvpn_arg (args, stmp);
@@ -2048,9 +2063,7 @@ main (int argc, char *argv[])
        g_option_context_free (opt_ctx);
 
        gl.log_level = _nm_utils_ascii_str_to_int64 (getenv ("NM_VPN_LOG_LEVEL"),
-                                                    10, 0, LOG_DEBUG,
-                                                    gl.debug ? LOG_DEBUG : 0);
-
+                                                    10, 0, LOG_DEBUG, -1);
        gl.log_syslog = _nm_utils_ascii_str_to_int64 (getenv ("NM_VPN_LOG_SYSLOG"),
                                                      10, 0, 1,
                                                      gl.debug ? 0 : 1);