[network-manager-libreswan/th/vpn-plugin-debug-bgo766872: 9/22] shared: refactor debug logging for nm_libreswan_config_write()

[Thomas Haller <thaller src gnome org>]

commit 0bf0e02ced407cae3a9f831d2cbc891680852916
Author: Thomas Haller <thaller redhat com>
Date:   Thu May 26 12:45:08 2016 +0200

    shared: refactor debug logging for nm_libreswan_config_write()
    
    Instead of checking the global variable @debug and issue a g_print()
    statement inside nm_libreswan_config_write()/write_config_option(),
    accept a function pointer @debug_write_fcn.
    
    The reason is two-fold: for one, printing a debug statement is only
    suitable for utils when linked against the core service, not from
    inside the properties plugin.
    
    When logging a message from core, we want to format them a certain
    way that is not available from inside the utility code (e.g. consider
    logging levels more complex than a global @debug variable, add
    a prefix to the logging message, possibly use syslog()).

 properties/nm-libreswan-editor-plugin.c |    2 +-
 shared/utils.c                          |   61 +++++++++++++++----------------
 shared/utils.h                          |   13 ++++---
 src/nm-libreswan-service.c              |   15 ++++++--
 4 files changed, 50 insertions(+), 41 deletions(-)
---
diff --git a/properties/nm-libreswan-editor-plugin.c b/properties/nm-libreswan-editor-plugin.c
index d152ecc..548ce33 100644
--- a/properties/nm-libreswan-editor-plugin.c
+++ b/properties/nm-libreswan-editor-plugin.c
@@ -154,7 +154,7 @@ export_to_file (NMVpnEditorPlugin *self,
        if (s_vpn)
                openswan = nm_streq (nm_setting_vpn_get_service_type (s_vpn), NM_VPN_SERVICE_TYPE_OPENSWAN);
 
-       if (!nm_libreswan_config_write (fd, connection, NULL, openswan, &local)) {
+       if (!nm_libreswan_config_write (fd, connection, NULL, openswan, NULL, &local)) {
                g_close (fd, NULL);
                g_set_error (error, NMV_EDITOR_PLUGIN_ERROR, NMV_EDITOR_PLUGIN_ERROR_FAILED,
                             _("Error writing to file '%s': %s"), path, local->message);
diff --git a/shared/utils.c b/shared/utils.c
index d6128f2..38977a9 100644
--- a/shared/utils.c
+++ b/shared/utils.c
@@ -28,13 +28,12 @@
 #include <unistd.h>
 #include <string.h>
 
-gboolean debug = FALSE;
-
 gboolean
 nm_libreswan_config_write (gint fd,
                            NMConnection *connection,
                            const char *bus_name,
                            gboolean openswan,
+                           NMDebugWriteFcn debug_write_fcn,
                            GError **error)
 {
        NMSettingVpn *s_vpn = nm_connection_get_setting_vpn (connection);
@@ -60,64 +59,64 @@ nm_libreswan_config_write (gint fd,
 
        leftid = nm_setting_vpn_get_data_item (s_vpn, NM_LIBRESWAN_LEFTID);
 
-#define WRITE_CHECK_NEWLINE(fd, new_line, error, ...) \
+#define WRITE_CHECK_NEWLINE(fd, new_line, debug_write_fcn, error, ...) \
        G_STMT_START { \
-               if (!write_config_option_newline ((fd), (new_line), (error), __VA_ARGS__)) \
+               if (!write_config_option_newline ((fd), (new_line), debug_write_fcn, (error), __VA_ARGS__)) \
                        return FALSE; \
        } G_STMT_END
-#define WRITE_CHECK(fd, error, ...) WRITE_CHECK_NEWLINE (fd, TRUE, error, __VA_ARGS__)
+#define WRITE_CHECK(fd, debug_write_fcn, error, ...) WRITE_CHECK_NEWLINE (fd, TRUE, debug_write_fcn, error, 
__VA_ARGS__)
 
-       WRITE_CHECK (fd, error, "conn %s", con_name);
+       WRITE_CHECK (fd, debug_write_fcn, error, "conn %s", con_name);
        if (leftid) {
-               WRITE_CHECK (fd, error, " aggrmode=yes");
-               WRITE_CHECK (fd, error, " leftid= %s", leftid);
+               WRITE_CHECK (fd, debug_write_fcn, error, " aggrmode=yes");
+               WRITE_CHECK (fd, debug_write_fcn, error, " leftid= %s", leftid);
        }
-       WRITE_CHECK (fd, error, " authby=secret");
-       WRITE_CHECK (fd, error, " left=%%defaultroute");
-       WRITE_CHECK (fd, error, " leftxauthclient=yes");
-       WRITE_CHECK (fd, error, " leftmodecfgclient=yes");
+       WRITE_CHECK (fd, debug_write_fcn, error, " authby=secret");
+       WRITE_CHECK (fd, debug_write_fcn, error, " left=%%defaultroute");
+       WRITE_CHECK (fd, debug_write_fcn, error, " leftxauthclient=yes");
+       WRITE_CHECK (fd, debug_write_fcn, error, " leftmodecfgclient=yes");
 
        if (bus_name)
-               WRITE_CHECK (fd, error, " leftupdown=\"" NM_LIBRESWAN_HELPER_PATH " --bus-name %s\"", 
bus_name);
+               WRITE_CHECK (fd, debug_write_fcn, error, " leftupdown=\"" NM_LIBRESWAN_HELPER_PATH " 
--bus-name %s\"", bus_name);
 
        default_username = nm_setting_vpn_get_user_name (s_vpn);
        props_username = nm_setting_vpn_get_data_item (s_vpn, NM_LIBRESWAN_LEFTXAUTHUSER);
        if (props_username && strlen (props_username))
-               WRITE_CHECK (fd, error, " leftxauthusername=%s", props_username);
+               WRITE_CHECK (fd, debug_write_fcn, error, " leftxauthusername=%s", props_username);
        else if (default_username && strlen (default_username))
-               WRITE_CHECK (fd, error, " leftxauthusername=%s", default_username);
+               WRITE_CHECK (fd, debug_write_fcn, error, " leftxauthusername=%s", default_username);
 
-       WRITE_CHECK (fd, error, " right=%s", nm_setting_vpn_get_data_item (s_vpn, NM_LIBRESWAN_RIGHT));
-       WRITE_CHECK (fd, error, " remote_peer_type=cisco");
-       WRITE_CHECK (fd, error, " rightxauthserver=yes");
-       WRITE_CHECK (fd, error, " rightmodecfgserver=yes");
-       WRITE_CHECK (fd, error, " modecfgpull=yes");
-       WRITE_CHECK (fd, error, " rightsubnet=0.0.0.0/0");
+       WRITE_CHECK (fd, debug_write_fcn, error, " right=%s", nm_setting_vpn_get_data_item (s_vpn, 
NM_LIBRESWAN_RIGHT));
+       WRITE_CHECK (fd, debug_write_fcn, error, " remote_peer_type=cisco");
+       WRITE_CHECK (fd, debug_write_fcn, error, " rightxauthserver=yes");
+       WRITE_CHECK (fd, debug_write_fcn, error, " rightmodecfgserver=yes");
+       WRITE_CHECK (fd, debug_write_fcn, error, " modecfgpull=yes");
+       WRITE_CHECK (fd, debug_write_fcn, error, " rightsubnet=0.0.0.0/0");
 
        phase1_alg_str = nm_setting_vpn_get_data_item (s_vpn, NM_LIBRESWAN_IKE);
        if (!phase1_alg_str || !strlen (phase1_alg_str))
-               WRITE_CHECK (fd, error, " ike=aes-sha1");
+               WRITE_CHECK (fd, debug_write_fcn, error, " ike=aes-sha1");
        else
-               WRITE_CHECK (fd, error, " ike=%s", phase1_alg_str);
+               WRITE_CHECK (fd, debug_write_fcn, error, " ike=%s", phase1_alg_str);
 
        phase2_alg_str = nm_setting_vpn_get_data_item (s_vpn, NM_LIBRESWAN_ESP);
        if (!phase2_alg_str || !strlen (phase2_alg_str))
-               WRITE_CHECK (fd, error, " esp=aes-sha1;modp1024");
+               WRITE_CHECK (fd, debug_write_fcn, error, " esp=aes-sha1;modp1024");
        else
-               WRITE_CHECK (fd, error, " esp=%s", phase2_alg_str);
+               WRITE_CHECK (fd, debug_write_fcn, error, " esp=%s", phase2_alg_str);
 
-       WRITE_CHECK (fd, error, " rekey=yes");
-       WRITE_CHECK (fd, error, " salifetime=24h");
-       WRITE_CHECK (fd, error, " ikelifetime=24h");
-       WRITE_CHECK (fd, error, " keyingtries=1");
+       WRITE_CHECK (fd, debug_write_fcn, error, " rekey=yes");
+       WRITE_CHECK (fd, debug_write_fcn, error, " salifetime=24h");
+       WRITE_CHECK (fd, debug_write_fcn, error, " ikelifetime=24h");
+       WRITE_CHECK (fd, debug_write_fcn, error, " keyingtries=1");
        if (!openswan && g_strcmp0 (nm_setting_vpn_get_data_item (s_vpn, NM_LIBRESWAN_VENDOR), "Cisco") == 0)
-               WRITE_CHECK (fd, error, " cisco-unity=yes");
+               WRITE_CHECK (fd, debug_write_fcn, error, " cisco-unity=yes");
 
        /* openswan requires a terminating \n (otherwise it segfaults) while
         * libreswan fails parsing the configuration if you include the \n.
         * WTF?
         */
-       WRITE_CHECK_NEWLINE (fd, (openswan || !bus_name), error, " auto=add");
+       WRITE_CHECK_NEWLINE (fd, (openswan || !bus_name), debug_write_fcn, error, " auto=add");
 
        return TRUE;
 }
diff --git a/shared/utils.h b/shared/utils.h
index 9a2c25a..6181218 100644
--- a/shared/utils.h
+++ b/shared/utils.h
@@ -26,11 +26,11 @@
 
 #include <errno.h>
 
-extern gboolean debug;
+typedef void (*NMDebugWriteFcn) (const char *setting);
 
-__attribute__((__format__ (__printf__, 4, 5)))
+__attribute__((__format__ (__printf__, 5, 6)))
 static inline gboolean
-write_config_option_newline (int fd, gboolean new_line, GError **error, const char *format, ...)
+write_config_option_newline (int fd, gboolean new_line, NMDebugWriteFcn debug_write_fcn, GError **error, 
const char *format, ...)
 {
        gs_free char *string = NULL;
        const char *p;
@@ -43,8 +43,8 @@ write_config_option_newline (int fd, gboolean new_line, GError **error, const ch
        string = g_strdup_vprintf (format, args);
        va_end (args);
 
-       if (debug)
-               g_print ("Config: %s\n", string);
+       if (debug_write_fcn)
+               debug_write_fcn (string);
 
        l = strlen (string);
        if (new_line) {
@@ -81,13 +81,14 @@ write_config_option_newline (int fd, gboolean new_line, GError **error, const ch
                     _("Error writing config: %s"), g_strerror (errsv));
        return FALSE;
 }
-#define write_config_option(fd, error, ...) write_config_option_newline((fd), TRUE, error, __VA_ARGS__)
+#define write_config_option(fd, debug_write_fcn, error, ...) write_config_option_newline((fd), TRUE, 
debug_write_fcn, error, __VA_ARGS__)
 
 gboolean
 nm_libreswan_config_write (gint fd,
                            NMConnection *connection,
                            const char *bus_name,
                            gboolean openswan,
+                           NMDebugWriteFcn debug_write_fcn,
                            GError **error);
 
 #endif /* __UTILS_H__ */
diff --git a/src/nm-libreswan-service.c b/src/nm-libreswan-service.c
index 2292906..742f9a7 100644
--- a/src/nm-libreswan-service.c
+++ b/src/nm-libreswan-service.c
@@ -71,6 +71,7 @@ G_DEFINE_TYPE (NMLibreswanPlugin, nm_libreswan_plugin, NM_TYPE_VPN_SERVICE_PLUGI
 /************************************************************/
 
 GMainLoop *loop = NULL;
+static gboolean debug;
 
 typedef enum {
     CONNECT_STEP_FIRST,
@@ -132,6 +133,14 @@ typedef struct {
 
 /****************************************************************/
 
+static void
+_debug_write_option (const char *setting)
+{
+       g_print ("Config %s\n", setting);
+}
+
+/****************************************************************/
+
 guint32
 nm_utils_ip4_prefix_to_netmask (guint32 prefix)
 {
@@ -695,11 +704,11 @@ nm_libreswan_config_psk_write (NMSettingVpn *s_vpn,
 
        leftid = nm_setting_vpn_get_data_item (s_vpn, NM_LIBRESWAN_LEFTID);
        if (leftid) {
-               success = write_config_option (fd, error, "@%s: PSK \"%s\"", leftid, psk);
+               success = write_config_option (fd, _debug_write_option, error, "@%s: PSK \"%s\"", leftid, 
psk);
        } else {
                right = nm_setting_vpn_get_data_item (s_vpn, NM_LIBRESWAN_RIGHT);
                g_assert (right);
-               success = write_config_option (fd, error, "%s %%any: PSK \"%s\"", right, psk);
+               success = write_config_option (fd, _debug_write_option, error, "%s %%any: PSK \"%s\"", right, 
psk);
        }
 
        if (!success) {
@@ -1603,7 +1612,7 @@ connect_step (NMLibreswanPlugin *self, GError **error)
                        return FALSE;
                priv->watch_id = g_child_watch_add (priv->pid, child_watch_cb, self);
                g_object_get (self, NM_VPN_SERVICE_PLUGIN_DBUS_SERVICE_NAME, &bus_name, NULL);
-               if (!nm_libreswan_config_write (fd, priv->connection, bus_name, priv->openswan, error)) {
+               if (!nm_libreswan_config_write (fd, priv->connection, bus_name, priv->openswan, 
_debug_write_option, error)) {
                        g_close (fd, NULL);
                        g_free (bus_name);
                        return FALSE;