ppp: replace NMCmdLine API with plain GPtrArray in create_pppd_cmd_line()

1 files changed, 85 insertions, 146 deletions

diff --git a/src/ppp/nm-ppp-manager.c b/src/ppp/nm-ppp-manager.c
index 1595961f4..d012fcb71 100644
--- a/src/ppp/nm-ppp-manager.c
+++ b/src/ppp/nm-ppp-manager.c
@@ -675,61 +675,6 @@ out:
 
 /*****************************************************************************/
 
-typedef struct {
-	GPtrArray *array;
-	GStringChunk *chunk;
-} NMCmdLine;
-
-static NMCmdLine *
-nm_cmd_line_new (void)
-{
-	NMCmdLine *cmd;
-
-	cmd = g_slice_new (NMCmdLine);
-	cmd->array = g_ptr_array_new ();
-	cmd->chunk = g_string_chunk_new (1024);
-
-	return cmd;
-}
-
-static void
-nm_cmd_line_destroy (NMCmdLine *cmd)
-{
-	g_ptr_array_free (cmd->array, TRUE);
-	g_string_chunk_free (cmd->chunk);
-	g_slice_free (NMCmdLine, cmd);
-}
-
-static char *
-nm_cmd_line_to_str (NMCmdLine *cmd)
-{
-	char *str;
-
-	g_ptr_array_add (cmd->array, NULL);
-	str = g_strjoinv (" ", (char **) cmd->array->pdata);
-	g_ptr_array_remove_index (cmd->array, cmd->array->len - 1);
-
-	return str;
-}
-
-static void
-nm_cmd_line_add_string (NMCmdLine *cmd, const char *str)
-{
-	g_ptr_array_add (cmd->array, g_string_chunk_insert (cmd->chunk, str));
-}
-
-static void
-nm_cmd_line_add_int (NMCmdLine *cmd, int i)
-{
-	char *str;
-
-	str = g_strdup_printf ("%d", i);
-	nm_cmd_line_add_string (cmd, str);
-	g_free (str);
-}
-
-/*****************************************************************************/
-
 NM_UTILS_LOOKUP_STR_DEFINE_STATIC (pppd_exit_code_to_str, int,
 	NM_UTILS_LOOKUP_DEFAULT ("Unknown error"),
 	NM_UTILS_LOOKUP_STR_ITEM ( 1, "Fatal pppd error");
@@ -801,7 +746,7 @@ pppd_timed_out (gpointer data)
 	return FALSE;
 }
 
-static NMCmdLine *
+static GPtrArray *
 create_pppd_cmd_line (NMPPPManager *self,
                       NMSettingPpp *setting,
                       NMSettingPppoe *pppoe,
@@ -814,9 +759,8 @@ create_pppd_cmd_line (NMPPPManager *self,
 {
 	NMPPPManagerPrivate *priv = NM_PPP_MANAGER_GET_PRIVATE (self);
 	const char *pppd_binary = NULL;
-	NMCmdLine *cmd;
+	gs_unref_ptrarray GPtrArray *cmd = NULL;
 	gboolean ppp_debug;
-	static int unit;
 
 	g_return_val_if_fail (setting != NULL, NULL);
 
@@ -836,53 +780,50 @@ create_pppd_cmd_line (NMPPPManager *self,
 		return NULL;
 	}
 
-	/* Create pppd command line */
-	cmd = nm_cmd_line_new ();
-	nm_cmd_line_add_string (cmd, pppd_binary);
+	cmd = g_ptr_array_new_with_free_func (g_free);
+
+	nm_strv_ptrarray_add_string_dup (cmd, pppd_binary);
 
-	nm_cmd_line_add_string (cmd, "nodetach");
-	nm_cmd_line_add_string (cmd, "lock");
+	nm_strv_ptrarray_add_string_dup (cmd, "nodetach");
+	nm_strv_ptrarray_add_string_dup (cmd, "lock");
 
 	/* NM handles setting the default route */
-	nm_cmd_line_add_string (cmd, "nodefaultroute");
+	nm_strv_ptrarray_add_string_dup (cmd, "nodefaultroute");
 
 	if (!ip4_enabled)
-		nm_cmd_line_add_string (cmd, "noip");
+		nm_strv_ptrarray_add_string_dup (cmd, "noip");
 
 	if (ip6_enabled) {
 		/* Allow IPv6 to be configured by IPV6CP */
-		nm_cmd_line_add_string (cmd, "ipv6");
-		nm_cmd_line_add_string (cmd, ",");
+		nm_strv_ptrarray_add_string_dup (cmd, "ipv6");
+		nm_strv_ptrarray_add_string_dup (cmd, ",");
 	} else
-		nm_cmd_line_add_string (cmd, "noipv6");
+		nm_strv_ptrarray_add_string_dup (cmd, "noipv6");
 
 	ppp_debug = !!getenv ("NM_PPP_DEBUG");
 	if (nm_logging_enabled (LOGL_DEBUG, LOGD_PPP))
 		ppp_debug = TRUE;
 
 	if (ppp_debug)
-		nm_cmd_line_add_string (cmd, "debug");
+		nm_strv_ptrarray_add_string_dup (cmd, "debug");
 
 	if (ppp_name) {
-		nm_cmd_line_add_string (cmd, "user");
-		nm_cmd_line_add_string (cmd, ppp_name);
+		nm_strv_ptrarray_add_string_dup (cmd, "user");
+		nm_strv_ptrarray_add_string_dup (cmd, ppp_name);
 	}
 
 	if (pppoe) {
-		char *dev_str;
 		const char *pppoe_service;
 
-		nm_cmd_line_add_string (cmd, "plugin");
-		nm_cmd_line_add_string (cmd, "rp-pppoe.so");
+		nm_strv_ptrarray_add_string_dup (cmd, "plugin");
+		nm_strv_ptrarray_add_string_dup (cmd, "rp-pppoe.so");
 
-		dev_str = g_strdup_printf ("nic-%s", priv->parent_iface);
-		nm_cmd_line_add_string (cmd, dev_str);
-		g_free (dev_str);
+		nm_strv_ptrarray_add_string_concat (cmd, "nic-", priv->parent_iface);
 
 		pppoe_service = nm_setting_pppoe_get_service (pppoe);
 		if (pppoe_service) {
-			nm_cmd_line_add_string (cmd, "rp_pppoe_service");
-			nm_cmd_line_add_string (cmd, pppoe_service);
+			nm_strv_ptrarray_add_string_dup (cmd, "rp_pppoe_service");
+			nm_strv_ptrarray_add_string_dup (cmd, pppoe_service);
 		}
 	} else if (adsl) {
 		const char *protocol = nm_setting_adsl_get_protocol (adsl);
@@ -891,110 +832,110 @@ create_pppd_cmd_line (NMPPPManager *self,
 			guint32 vpi = nm_setting_adsl_get_vpi (adsl);
 			guint32 vci = nm_setting_adsl_get_vci (adsl);
 			const char *encaps = nm_setting_adsl_get_encapsulation (adsl);
-			char *vpivci;
 
-			nm_cmd_line_add_string (cmd, "plugin");
-			nm_cmd_line_add_string (cmd, "pppoatm.so");
+			nm_strv_ptrarray_add_string_dup (cmd, "plugin");
+			nm_strv_ptrarray_add_string_dup (cmd, "pppoatm.so");
 
-			vpivci = g_strdup_printf("%d.%d", vpi, vci);
-			nm_cmd_line_add_string (cmd, vpivci);
-			g_free (vpivci);
+			nm_strv_ptrarray_add_string_printf (cmd, "%d.%d", vpi, vci);
 
 			if (g_strcmp0 (encaps, NM_SETTING_ADSL_ENCAPSULATION_LLC) == 0)
-				nm_cmd_line_add_string (cmd, "llc-encaps");
+				nm_strv_ptrarray_add_string_dup (cmd, "llc-encaps");
 			else /*if (g_strcmp0 (encaps, NM_SETTING_ADSL_ENCAPSULATION_VCMUX) == 0)*/
-				nm_cmd_line_add_string (cmd, "vc-encaps");
+				nm_strv_ptrarray_add_string_dup (cmd, "vc-encaps");
 
 		} else if (!strcmp (protocol, NM_SETTING_ADSL_PROTOCOL_PPPOE)) {
-			nm_cmd_line_add_string (cmd, "plugin");
-			nm_cmd_line_add_string (cmd, "rp-pppoe.so");
-			nm_cmd_line_add_string (cmd, priv->parent_iface);
+			nm_strv_ptrarray_add_string_dup (cmd, "plugin");
+			nm_strv_ptrarray_add_string_dup (cmd, "rp-pppoe.so");
+			nm_strv_ptrarray_add_string_dup (cmd, priv->parent_iface);
 		}
 
-		nm_cmd_line_add_string (cmd, "noipdefault");
+		nm_strv_ptrarray_add_string_dup (cmd, "noipdefault");
 	} else {
-		nm_cmd_line_add_string (cmd, priv->parent_iface);
+		nm_strv_ptrarray_add_string_dup (cmd, priv->parent_iface);
 		/* Don't send some random address as the local address */
-		nm_cmd_line_add_string (cmd, "noipdefault");
+		nm_strv_ptrarray_add_string_dup (cmd, "noipdefault");
 	}
 
 	if (nm_setting_ppp_get_baud (setting))
-		nm_cmd_line_add_int (cmd, nm_setting_ppp_get_baud (setting));
+		nm_strv_ptrarray_add_int (cmd, nm_setting_ppp_get_baud (setting));
 	else if (baud_override)
-		nm_cmd_line_add_int (cmd, (int) baud_override);
+		nm_strv_ptrarray_add_int (cmd, baud_override);
 
 	/* noauth by default, because we certainly don't have any information
 	 * with which to verify anything the peer gives us if we ask it to
 	 * authenticate itself, which is what 'auth' really means.
 	 */
-	nm_cmd_line_add_string (cmd, "noauth");
+	nm_strv_ptrarray_add_string_dup (cmd, "noauth");
 
 	if (nm_setting_ppp_get_refuse_eap (setting))
-		nm_cmd_line_add_string (cmd, "refuse-eap");
+		nm_strv_ptrarray_add_string_dup (cmd, "refuse-eap");
 	if (nm_setting_ppp_get_refuse_pap (setting))
-		nm_cmd_line_add_string (cmd, "refuse-pap");
+		nm_strv_ptrarray_add_string_dup (cmd, "refuse-pap");
 	if (nm_setting_ppp_get_refuse_chap (setting))
-		nm_cmd_line_add_string (cmd, "refuse-chap");
+		nm_strv_ptrarray_add_string_dup (cmd, "refuse-chap");
 	if (nm_setting_ppp_get_refuse_mschap (setting))
-		nm_cmd_line_add_string (cmd, "refuse-mschap");
+		nm_strv_ptrarray_add_string_dup (cmd, "refuse-mschap");
 	if (nm_setting_ppp_get_refuse_mschapv2 (setting))
-		nm_cmd_line_add_string (cmd, "refuse-mschap-v2");
+		nm_strv_ptrarray_add_string_dup (cmd, "refuse-mschap-v2");
 	if (nm_setting_ppp_get_nobsdcomp (setting))
-		nm_cmd_line_add_string (cmd, "nobsdcomp");
+		nm_strv_ptrarray_add_string_dup (cmd, "nobsdcomp");
 	if (nm_setting_ppp_get_no_vj_comp (setting))
-		nm_cmd_line_add_string (cmd, "novj");
+		nm_strv_ptrarray_add_string_dup (cmd, "novj");
 	if (nm_setting_ppp_get_nodeflate (setting))
-		nm_cmd_line_add_string (cmd, "nodeflate");
+		nm_strv_ptrarray_add_string_dup (cmd, "nodeflate");
 	if (nm_setting_ppp_get_require_mppe (setting))
-		nm_cmd_line_add_string (cmd, "require-mppe");
+		nm_strv_ptrarray_add_string_dup (cmd, "require-mppe");
 	if (nm_setting_ppp_get_require_mppe_128 (setting))
-		nm_cmd_line_add_string (cmd, "require-mppe-128");
+		nm_strv_ptrarray_add_string_dup (cmd, "require-mppe-128");
 	if (nm_setting_ppp_get_mppe_stateful (setting))
-		nm_cmd_line_add_string (cmd, "mppe-stateful");
+		nm_strv_ptrarray_add_string_dup (cmd, "mppe-stateful");
 	if (nm_setting_ppp_get_crtscts (setting))
-		nm_cmd_line_add_string (cmd, "crtscts");
+		nm_strv_ptrarray_add_string_dup (cmd, "crtscts");
 
 	/* Always ask for DNS, we don't have to use them if the connection
 	 * overrides the returned servers.
 	 */
-	nm_cmd_line_add_string (cmd, "usepeerdns");
+	nm_strv_ptrarray_add_string_dup (cmd, "usepeerdns");
 
 	if (nm_setting_ppp_get_mru (setting)) {
-		nm_cmd_line_add_string (cmd, "mru");
-		nm_cmd_line_add_int (cmd, nm_setting_ppp_get_mru (setting));
+		nm_strv_ptrarray_add_string_dup (cmd, "mru");
+		nm_strv_ptrarray_add_int (cmd, nm_setting_ppp_get_mru (setting));
 	}
 
 	if (nm_setting_ppp_get_mtu (setting)) {
-		nm_cmd_line_add_string (cmd, "mtu");
-		nm_cmd_line_add_int (cmd, nm_setting_ppp_get_mtu (setting));
+		nm_strv_ptrarray_add_string_dup (cmd, "mtu");
+		nm_strv_ptrarray_add_int (cmd, nm_setting_ppp_get_mtu (setting));
 	}
 
-	nm_cmd_line_add_string (cmd, "lcp-echo-failure");
-	nm_cmd_line_add_int (cmd, nm_setting_ppp_get_lcp_echo_failure (setting));
+	nm_strv_ptrarray_add_string_dup (cmd, "lcp-echo-failure");
+	nm_strv_ptrarray_add_int (cmd, nm_setting_ppp_get_lcp_echo_failure (setting));
 
-	nm_cmd_line_add_string (cmd, "lcp-echo-interval");
-	nm_cmd_line_add_int (cmd, nm_setting_ppp_get_lcp_echo_interval (setting));
+	nm_strv_ptrarray_add_string_dup (cmd, "lcp-echo-interval");
+	nm_strv_ptrarray_add_int (cmd, nm_setting_ppp_get_lcp_echo_interval (setting));
 
 	/* Avoid pppd to exit if no traffic going through */
-	nm_cmd_line_add_string (cmd, "idle");
-	nm_cmd_line_add_int (cmd, 0);
+	nm_strv_ptrarray_add_string_dup (cmd, "idle");
+	nm_strv_ptrarray_add_string_dup (cmd, "0");
 
-	nm_cmd_line_add_string (cmd, "ipparam");
-	nm_cmd_line_add_string (cmd, nm_dbus_object_get_path (NM_DBUS_OBJECT (self)));
+	nm_strv_ptrarray_add_string_dup (cmd, "ipparam");
+	nm_strv_ptrarray_add_string_dup (cmd, nm_dbus_object_get_path (NM_DBUS_OBJECT (self)));
 
-	nm_cmd_line_add_string (cmd, "plugin");
-	nm_cmd_line_add_string (cmd, NM_PPPD_PLUGIN);
+	nm_strv_ptrarray_add_string_dup (cmd, "plugin");
+	nm_strv_ptrarray_add_string_dup (cmd, NM_PPPD_PLUGIN);
 
 	if (pppoe && nm_setting_pppoe_get_parent (pppoe)) {
+		static int unit;
+
 		/* The PPP interface is going to be renamed, so pass a
 		 * different unit each time so that activations don't
 		 * race with each others. */
-		nm_cmd_line_add_string (cmd, "unit");
-		nm_cmd_line_add_int (cmd, unit);
+		nm_strv_ptrarray_add_string_dup (cmd, "unit");
+		nm_strv_ptrarray_add_int (cmd, unit);
 		unit = unit < G_MAXINT ? unit + 1 : 0;
 	}
 
-	return cmd;
+	g_ptr_array_add (cmd, NULL);
+	return g_steal_pointer (&cmd);
 }
 
 static void
@@ -1038,8 +979,8 @@ _ppp_manager_start (NMPPPManager *self,
 	gs_unref_object NMSettingPpp *s_ppp_free = NULL;
 	NMSettingPppoe *pppoe_setting;
 	NMSettingAdsl *adsl_setting;
-	NMCmdLine *ppp_cmd;
-	char *cmd_str;
+	gs_unref_ptrarray GPtrArray *ppp_cmd = NULL;
+	gs_free char *cmd_str = NULL;
 	struct stat st;
 	const char *ip6_method, *ip4_method;
 	gboolean ip6_enabled = FALSE;
@@ -1104,23 +1045,25 @@ _ppp_manager_start (NMPPPManager *self,
 	                                ip6_enabled,
 	                                err);
 	if (!ppp_cmd)
-		goto out;
-
-	g_ptr_array_add (ppp_cmd->array, NULL);
+		goto fail;
 
 	_LOGI ("starting PPP connection");
 
-	cmd_str = nm_cmd_line_to_str (ppp_cmd);
 	_LOGD ("command line: %s", cmd_str);
-	g_free (cmd_str);
+	       (cmd_str = g_strjoinv (" ", (char **) ppp_cmd->pdata));
 
 	priv->pid = 0;
-	if (!g_spawn_async (NULL, (char **) ppp_cmd->array->pdata, NULL,
+	if (!g_spawn_async (NULL,
+	                    (char **) ppp_cmd->pdata,
+	                    NULL,
 	                    G_SPAWN_DO_NOT_REAP_CHILD,
-	                    nm_utils_setpgid, NULL,
-	                    &priv->pid, err)) {
-		goto out;
-	}
+	                    nm_utils_setpgid,
+	                    NULL,
+	                    &priv->pid,
+	                    err))
+		goto fail;
+
+	nm_assert (priv->pid > 0);
 
 	_LOGI ("pppd started with pid %lld", (long long) priv->pid);
 
@@ -1128,14 +1071,10 @@ _ppp_manager_start (NMPPPManager *self,
 	priv->ppp_timeout_handler = g_timeout_add_seconds (timeout_secs, pppd_timed_out, self);
 	priv->act_req = g_object_ref (req);
 
-out:
-	if (ppp_cmd)
-		nm_cmd_line_destroy (ppp_cmd);
-
-	if (priv->pid <= 0)
-		nm_dbus_object_unexport (NM_DBUS_OBJECT (self));
-
-	return priv->pid > 0;
+	return TRUE;
+fail:
+	nm_dbus_object_unexport (NM_DBUS_OBJECT (self));
+	return FALSE;
 }
 
 static void