[network-manager-libreswan/th/vpn-plugin-debug-bgo766872: 16/20] shared: refactor arguments for nm_libreswan_config_write()

[Thomas Haller <thaller src gnome org>]

commit 6e089dd8ae00132153928519d99e87413a2a4113
Author: Thomas Haller <thaller redhat com>
Date:   Thu May 26 17:36:35 2016 +0200

    shared: refactor arguments for nm_libreswan_config_write()
    
    Before, nm_libreswan_config_write() had fewer arguments, and based on those arguments
    nm_libreswan_config_write() would decide what to do. For example, depending on the
    presence of @bus_name, it would choose as connection name the ID or the
    UUID.
    
    Refactor the arguments, to move logic out of nm_libreswan_config_write():
    
     - instead of detecting the @con_name based on the presense of @bus_name,
       have an explict argument.
     - instead of deciding whether to append a @trailing_newline based on
       (openswan || !bus_name), let the caller decide.
     - the leftupdown_script is build based on @bus_name.
    
    The reason for this change is, that with the next patch, we want to
    pass more argument to the leftupdown script. Thus, we anyway would need
    a to add a @leftupdown_script argument for that. But then keeping @bus_name
    to control @con_name and @trailing_newline is confusing.

 properties/nm-libreswan-editor-plugin.c |    9 +++++++-
 shared/utils.c                          |   33 ++++++++++++------------------
 shared/utils.h                          |    4 ++-
 src/nm-libreswan-service.c              |   31 ++++++++++++++++++++++------
 4 files changed, 48 insertions(+), 29 deletions(-)
---
diff --git a/properties/nm-libreswan-editor-plugin.c b/properties/nm-libreswan-editor-plugin.c
index 91cdbc7..ca7fb87 100644
--- a/properties/nm-libreswan-editor-plugin.c
+++ b/properties/nm-libreswan-editor-plugin.c
@@ -154,7 +154,14 @@ 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, NULL, &local)) {
+       if (!nm_libreswan_config_write (fd,
+                                       connection,
+                                       nm_connection_get_id (connection),
+                                       NULL,
+                                       openswan,
+                                       TRUE,
+                                       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 fe311a0..ca64dbc 100644
--- a/shared/utils.c
+++ b/shared/utils.c
@@ -89,31 +89,27 @@ write_config_option_newline (int fd,
 gboolean
 nm_libreswan_config_write (gint fd,
                            NMConnection *connection,
-                           const char *bus_name,
+                           const char *con_name,
+                           const char *leftupdown_script,
                            gboolean openswan,
+                           gboolean trailing_newline,
                            NMDebugWriteFcn debug_write_fcn,
                            GError **error)
 {
-       NMSettingVpn *s_vpn = nm_connection_get_setting_vpn (connection);
-       const char *con_name;
+       NMSettingVpn *s_vpn;
        const char *props_username;
        const char *default_username;
        const char *phase1_alg_str;
        const char *phase2_alg_str;
        const char *leftid;
 
+       g_return_val_if_fail (fd > 0, FALSE);
+       g_return_val_if_fail (NM_IS_CONNECTION (connection), FALSE);
        g_return_val_if_fail (!error || !*error, FALSE);
+       g_return_val_if_fail (con_name && *con_name, FALSE);
 
-       /* We abuse the presence of bus name to decide if we're exporting
-        * the connection or actually configuring Pluto. */
-       if (bus_name)
-               con_name = nm_connection_get_uuid (connection);
-       else
-               con_name = nm_connection_get_id (connection);
-
-       g_assert (fd >= 0);
-       g_assert (s_vpn);
-       g_assert (con_name);
+       s_vpn = nm_connection_get_setting_vpn (connection);
+       g_return_val_if_fail (NM_IS_SETTING_VPN (s_vpn), FALSE);
 
        leftid = nm_setting_vpn_get_data_item (s_vpn, NM_LIBRESWAN_LEFTID);
 
@@ -134,8 +130,8 @@ nm_libreswan_config_write (gint fd,
        WRITE_CHECK (fd, debug_write_fcn, error, " leftxauthclient=yes");
        WRITE_CHECK (fd, debug_write_fcn, error, " leftmodecfgclient=yes");
 
-       if (bus_name)
-               WRITE_CHECK (fd, debug_write_fcn, error, " leftupdown=\"" NM_LIBRESWAN_HELPER_PATH " 
--bus-name %s\"", bus_name);
+       if (leftupdown_script)
+               WRITE_CHECK (fd, debug_write_fcn, error, " leftupdown=%s", leftupdown_script);
 
        default_username = nm_setting_vpn_get_user_name (s_vpn);
        props_username = nm_setting_vpn_get_data_item (s_vpn, NM_LIBRESWAN_LEFTXAUTHUSER);
@@ -167,14 +163,11 @@ nm_libreswan_config_write (gint fd,
        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, 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), debug_write_fcn, error, " auto=add");
+       WRITE_CHECK_NEWLINE (fd, trailing_newline, debug_write_fcn, error, " auto=add");
 
        return TRUE;
 }
diff --git a/shared/utils.h b/shared/utils.h
index 768a74a..eef444f 100644
--- a/shared/utils.h
+++ b/shared/utils.h
@@ -38,8 +38,10 @@ gboolean write_config_option_newline (int fd,
 gboolean
 nm_libreswan_config_write (gint fd,
                            NMConnection *connection,
-                           const char *bus_name,
+                           const char *con_name,
+                           const char *leftupdown_script,
                            gboolean openswan,
+                           gboolean trailing_newline,
                            NMDebugWriteFcn debug_write_fcn,
                            GError **error);
 
diff --git a/src/nm-libreswan-service.c b/src/nm-libreswan-service.c
index 70acc67..ebea977 100644
--- a/src/nm-libreswan-service.c
+++ b/src/nm-libreswan-service.c
@@ -1544,7 +1544,6 @@ connect_step (NMLibreswanPlugin *self, GError **error)
        const char *uuid;
        int fd = -1, up_stdout = -1, up_stderr = -1, up_pty = -1;
        gboolean success = FALSE;
-       char *bus_name;
 
        g_warn_if_fail (priv->watch_id == 0);
        priv->watch_id = 0;
@@ -1554,6 +1553,7 @@ connect_step (NMLibreswanPlugin *self, GError **error)
        _LOGD ("Connect: step %d", priv->connect_step);
 
        uuid = nm_connection_get_uuid (priv->connection);
+       g_return_val_if_fail (uuid && *uuid, FALSE);
 
        switch (priv->connect_step) {
        case CONNECT_STEP_FIRST:
@@ -1618,21 +1618,38 @@ connect_step (NMLibreswanPlugin *self, GError **error)
                priv->watch_id = g_child_watch_add (priv->pid, child_watch_cb, self);
                return TRUE;
 
-       case CONNECT_STEP_CONFIG_ADD:
-               g_assert (uuid);
+       case CONNECT_STEP_CONFIG_ADD: {
+               gboolean trailing_newline;
+               gs_free char *bus_name = NULL;
+               gs_free char *ifupdown_script = NULL;
+
                if (!do_spawn (self, &priv->pid, &fd, NULL, error, priv->ipsec_path,
                               "auto", "--replace", "--config", "-", uuid, NULL))
                        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, 
_debug_write_option, error)) {
+
+               /* openswan requires a terminating \n (otherwise it segfaults) while
+                * libreswan fails parsing the configuration if you include the \n.
+                * WTF?
+                */
+               trailing_newline = priv->openswan;
+
+               ifupdown_script = g_strdup_printf ("\"%s --bus-name %s\"", NM_LIBRESWAN_HELPER_PATH, 
bus_name);
+
+               if (!nm_libreswan_config_write (fd,
+                                               priv->connection,
+                                               uuid,
+                                               ifupdown_script,
+                                               priv->openswan,
+                                               trailing_newline,
+                                               _debug_write_option,
+                                               error)) {
                        g_close (fd, NULL);
-                       g_free (bus_name);
                        return FALSE;
                }
-               g_free (bus_name);
                return g_close (fd, error);
-
+       }
        case CONNECT_STEP_CONNECT:
                g_assert (uuid);
                if (!spawn_pty (self, &up_stdout, &up_stderr, &up_pty, &priv->pid, error,