shared: merge branch 'th/metered-for-shared'

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/57

7 files changed, 265 insertions, 254 deletions

diff --git a/shared/nm-utils/nm-shared-utils.h b/shared/nm-utils/nm-shared-utils.h
index 940a193f7..e82b53f7c 100644
--- a/shared/nm-utils/nm-shared-utils.h
+++ b/shared/nm-utils/nm-shared-utils.h
@@ -296,6 +296,37 @@ nm_memdup (gconstpointer data, gsize size)
 
 /*****************************************************************************/
 
+/* generic macro to convert an int to a (heap allocated) string.
+ *
+ * Usually, an inline function nm_strdup_int64() would be enough. However,
+ * that cannot be used for guint64. So, we would also need nm_strdup_uint64().
+ * This causes suble error potential, because the caller needs to ensure to
+ * use the right one (and compiler isn't going to help as it silently casts).
+ *
+ * Instead, this generic macro is supposed to handle all integers correctly. */
+#if _NM_CC_SUPPORT_GENERIC
+#define nm_strdup_int(val) \
+	_Generic ((val), \
+	          gint8:   g_strdup_printf ("%d",                (int)     (val)), \
+	          gint16:  g_strdup_printf ("%d",                (int)     (val)), \
+	          gint32:  g_strdup_printf ("%d",                (int)     (val)), \
+	          gint64:  g_strdup_printf ("%"G_GINT64_FORMAT,  (gint64)  (val)), \
+	          \
+	          guint8:  g_strdup_printf ("%u",                (guint)   (val)), \
+	          guint16: g_strdup_printf ("%u",                (guint)   (val)), \
+	          guint32: g_strdup_printf ("%u",                (guint)   (val)), \
+	          guint64: g_strdup_printf ("%"G_GUINT64_FORMAT, (guint64) (val))  \
+	)
+#else
+#define nm_strdup_int(val) \
+	(  (   sizeof (val) == sizeof (guint64) \
+	    && ((typeof (val)) -1) > 0) \
+	 ? g_strdup_printf ("%"G_GUINT64_FORMAT, (guint64) (val)) \
+	 : g_strdup_printf ("%"G_GINT64_FORMAT, (gint64) (val)))
+#endif
+
+/*****************************************************************************/
+
 extern const void *const _NM_PTRARRAY_EMPTY[1];
 
 #define NM_PTRARRAY_EMPTY(type) ((type const*) _NM_PTRARRAY_EMPTY)
@@ -959,4 +990,44 @@ void nm_utils_invoke_on_idle (NMUtilsInvokeOnIdleCallback callback,
                               gpointer callback_user_data,
                               GCancellable *cancellable);
 
+/*****************************************************************************/
+
+static inline void
+nm_strv_ptrarray_add_string_take (GPtrArray *cmd,
+                                  char *str)
+{
+	nm_assert (cmd);
+	nm_assert (str);
+
+	g_ptr_array_add (cmd, str);
+}
+
+static inline void
+nm_strv_ptrarray_add_string_dup (GPtrArray *cmd,
+                                 const char *str)
+{
+	nm_strv_ptrarray_add_string_take (cmd,
+	                                  g_strdup (str));
+}
+
+#define nm_strv_ptrarray_add_string_concat(cmd, ...) \
+	nm_strv_ptrarray_add_string_take ((cmd), g_strconcat (__VA_ARGS__, NULL))
+
+#define nm_strv_ptrarray_add_string_printf(cmd, ...) \
+	nm_strv_ptrarray_add_string_take ((cmd), g_strdup_printf (__VA_ARGS__))
+
+#define nm_strv_ptrarray_add_int(cmd, val) \
+	nm_strv_ptrarray_add_string_take ((cmd), nm_strdup_int (val))
+
+static inline void
+nm_strv_ptrarray_take_gstring (GPtrArray *cmd,
+                               GString **gstr)
+{
+	nm_assert (gstr && *gstr);
+
+	nm_strv_ptrarray_add_string_take (cmd,
+	                                  g_string_free (g_steal_pointer (gstr),
+	                                                 FALSE));
+}
+
 #endif /* __NM_SHARED_UTILS_H__ */
diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c
index 57021e4b7..c2223a985 100644
--- a/src/devices/nm-device.c
+++ b/src/devices/nm-device.c
@@ -10175,6 +10175,9 @@ start_sharing (NMDevice *self, NMIP4Config *config, GError **error)
 	const NMPlatformIP4Address *ip4_addr = NULL;
 	const char *ip_iface;
 	GError *local = NULL;
+	NMConnection *conn;
+	NMSettingConnection *s_con;
+	gboolean announce_android_metered;
 
 	g_return_val_if_fail (config, FALSE);
 
@@ -10196,7 +10199,7 @@ start_sharing (NMDevice *self, NMIP4Config *config, GError **error)
 		return FALSE;
 
 	req = nm_device_get_act_request (self);
-	g_assert (req);
+	g_return_val_if_fail (req, FALSE);
 
 	netmask = _nm_utils_ip4_prefix_to_netmask (ip4_addr->plen);
 	nm_utils_inet4_ntop (netmask, str_mask);
@@ -10217,7 +10220,35 @@ start_sharing (NMDevice *self, NMIP4Config *config, GError **error)
 
 	nm_act_request_set_shared (req, TRUE);
 
-	if (!nm_dnsmasq_manager_start (priv->dnsmasq_manager, config, &local)) {
+	conn = nm_act_request_get_applied_connection (req);
+	s_con = nm_connection_get_setting_connection (conn);
+
+	switch (nm_setting_connection_get_metered (s_con)) {
+	case NM_METERED_YES:
+		/* honor the metered flag. Note that reapply on the device does not affect
+		 * the metered setting. This is different from other profiles, where the
+		 * metered flag of an activated profile can be changed (reapplied). */
+		announce_android_metered = TRUE;
+		break;
+	case NM_METERED_UNKNOWN:
+		/* we pick up the current value and announce it. But again, we cannot update
+		 * the announced setting without restarting dnsmasq. That means, if the default
+		 * route changes w.r.t. being metered, then the shared connection does not get
+		 * updated before reactivating. */
+		announce_android_metered = NM_IN_SET (nm_manager_get_metered (nm_manager_get ()),
+		                                      NM_METERED_YES,
+		                                      NM_METERED_GUESS_YES);
+		break;
+	default:
+		announce_android_metered = FALSE;
+		break;
+	}
+
+
+	if (!nm_dnsmasq_manager_start (priv->dnsmasq_manager,
+	                               config,
+	                               announce_android_metered,
+	                               &local)) {
 		g_set_error (error, NM_UTILS_ERROR, NM_UTILS_ERROR_UNKNOWN,
 		             "could not start dnsmasq due to %s", local->message);
 		g_error_free (local);
diff --git a/src/dnsmasq/nm-dnsmasq-manager.c b/src/dnsmasq/nm-dnsmasq-manager.c
index 3fe2f4892..1afc6e0d7 100644
--- a/src/dnsmasq/nm-dnsmasq-manager.c
+++ b/src/dnsmasq/nm-dnsmasq-manager.c
@@ -73,51 +73,6 @@ G_DEFINE_TYPE (NMDnsMasqManager, nm_dnsmasq_manager, G_TYPE_OBJECT)
 
 /*****************************************************************************/
 
-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
 dm_watch_cb (GPid pid, int status, gpointer user_data)
 {
@@ -145,40 +100,40 @@ dm_watch_cb (GPid pid, int status, gpointer user_data)
 	g_signal_emit (manager, signals[STATE_CHANGED], 0, NM_DNSMASQ_STATUS_DEAD);
 }
 
-static NMCmdLine *
+static GPtrArray *
 create_dm_cmd_line (const char *iface,
                     const NMIP4Config *ip4_config,
                     const char *pidfile,
+                    gboolean announce_android_metered,
                     GError **error)
 {
-	NMCmdLine *cmd;
+	gs_unref_ptrarray GPtrArray *cmd = NULL;
 	nm_auto_free_gstring GString *s = NULL;
 	char first[INET_ADDRSTRLEN];
 	char last[INET_ADDRSTRLEN];
-	char localaddr[INET_ADDRSTRLEN];
+	char listen_address_s[INET_ADDRSTRLEN];
 	char tmpaddr[INET_ADDRSTRLEN];
-	char *error_desc = NULL;
+	gs_free char *error_desc = NULL;
 	const char *dm_binary;
 	const NMPlatformIP4Address *listen_address;
 	guint i, n;
 
 	listen_address = nm_ip4_config_get_first_address (ip4_config);
+
 	g_return_val_if_fail (listen_address, NULL);
 
 	dm_binary = nm_utils_find_helper ("dnsmasq", DNSMASQ_PATH, error);
 	if (!dm_binary)
 		return NULL;
 
-	s = g_string_sized_new (100);
+	cmd = g_ptr_array_new_with_free_func (g_free);
 
-	/* Create dnsmasq command line */
-	cmd = nm_cmd_line_new ();
-	nm_cmd_line_add_string (cmd, dm_binary);
+	nm_strv_ptrarray_add_string_dup (cmd, dm_binary);
 
 	if (   nm_logging_enabled (LOGL_TRACE, LOGD_SHARING)
 	    || getenv ("NM_DNSMASQ_DEBUG")) {
-		nm_cmd_line_add_string (cmd, "--log-dhcp");
-		nm_cmd_line_add_string (cmd, "--log-queries");
+		nm_strv_ptrarray_add_string_dup (cmd, "--log-dhcp");
+		nm_strv_ptrarray_add_string_dup (cmd, "--log-queries");
 	}
 
 	/* dnsmasq may read from its default config file location, which if that
@@ -187,25 +142,23 @@ create_dm_cmd_line (const char *iface,
 	 * as the gateway or whatever.  So tell dnsmasq not to use any config file
 	 * at all.
 	 */
-	nm_cmd_line_add_string (cmd, "--conf-file=/dev/null");
+	nm_strv_ptrarray_add_string_dup (cmd, "--conf-file=/dev/null");
 
-	nm_cmd_line_add_string (cmd, "--no-hosts");
-	nm_cmd_line_add_string (cmd, "--keep-in-foreground");
-	nm_cmd_line_add_string (cmd, "--bind-interfaces");
-	nm_cmd_line_add_string (cmd, "--except-interface=lo");
-	nm_cmd_line_add_string (cmd, "--clear-on-reload");
+	nm_strv_ptrarray_add_string_dup (cmd, "--no-hosts");
+	nm_strv_ptrarray_add_string_dup (cmd, "--keep-in-foreground");
+	nm_strv_ptrarray_add_string_dup (cmd, "--bind-interfaces");
+	nm_strv_ptrarray_add_string_dup (cmd, "--except-interface=lo");
+	nm_strv_ptrarray_add_string_dup (cmd, "--clear-on-reload");
 
 	/* Use strict order since in the case of VPN connections, the VPN's
 	 * nameservers will be first in resolv.conf, and those need to be tried
 	 * first by dnsmasq to successfully resolve names from the VPN.
 	 */
-	nm_cmd_line_add_string (cmd, "--strict-order");
+	nm_strv_ptrarray_add_string_dup (cmd, "--strict-order");
+
+	nm_utils_inet4_ntop (listen_address->address, listen_address_s);
 
-	nm_utils_inet4_ntop (listen_address->address, localaddr);
-	g_string_append (s, "--listen-address=");
-	g_string_append (s, localaddr);
-	nm_cmd_line_add_string (cmd, s->str);
-	g_string_truncate (s, 0);
+	nm_strv_ptrarray_add_string_concat (cmd, "--listen-address=", listen_address_s);
 
 	if (!nm_dnsmasq_utils_get_range (listen_address, first, last, &error_desc)) {
 		g_set_error_literal (error,
@@ -213,59 +166,61 @@ create_dm_cmd_line (const char *iface,
 		                     NM_MANAGER_ERROR_FAILED,
 		                     error_desc);
 		_LOGW ("failed to find DHCP address ranges: %s", error_desc);
-		g_free (error_desc);
-		nm_cmd_line_destroy (cmd);
 		return NULL;
 	}
 
-	g_string_append_printf (s, "--dhcp-range=%s,%s,60m", first, last);
-	nm_cmd_line_add_string (cmd, s->str);
-	g_string_truncate (s, 0);
+	nm_strv_ptrarray_add_string_printf (cmd,
+	                                    "--dhcp-range=%s,%s,60m",
+	                                    first,
+	                                    last);
 
 	if (nm_ip4_config_best_default_route_get (ip4_config)) {
-		g_string_append (s, "--dhcp-option=option:router,");
-		g_string_append (s, localaddr);
-		nm_cmd_line_add_string (cmd, s->str);
-		g_string_truncate (s, 0);
+		nm_strv_ptrarray_add_string_concat (cmd,
+		                                    "--dhcp-option=option:router,",
+		                                    listen_address_s);
 	}
 
 	if ((n = nm_ip4_config_get_num_nameservers (ip4_config))) {
+		nm_gstring_prepare (&s);
 		g_string_append (s, "--dhcp-option=option:dns-server");
 		for (i = 0; i < n; i++) {
 			g_string_append_c (s, ',');
 			g_string_append (s, nm_utils_inet4_ntop (nm_ip4_config_get_nameserver (ip4_config, i), tmpaddr));
 		}
-		nm_cmd_line_add_string (cmd, s->str);
-		g_string_truncate (s, 0);
+		nm_strv_ptrarray_take_gstring (cmd, &s);
 	}
 
 	if ((n = nm_ip4_config_get_num_searches (ip4_config))) {
+		nm_gstring_prepare (&s);
 		g_string_append (s, "--dhcp-option=option:domain-search");
 		for (i = 0; i < n; i++) {
 			g_string_append_c (s, ',');
 			g_string_append (s, nm_ip4_config_get_search (ip4_config, i));
 		}
-		nm_cmd_line_add_string (cmd, s->str);
-		g_string_truncate (s, 0);
+		nm_strv_ptrarray_take_gstring (cmd, &s);
 	}
 
-	nm_cmd_line_add_string (cmd, "--dhcp-lease-max=50");
+	if (announce_android_metered) {
+		/* force option 43 to announce ANDROID_METERED. Do this, even if the client
+		 * did not ask for this option. See https://www.lorier.net/docs/android-metered.html */
+		nm_strv_ptrarray_add_string_dup (cmd, "--dhcp-option-force=43,ANDROID_METERED");
+	}
+
+	nm_strv_ptrarray_add_string_dup (cmd, "--dhcp-lease-max=50");
 
-	g_string_append (s, "--dhcp-leasefile=" NMSTATEDIR);
-	g_string_append_printf (s, "/dnsmasq-%s.leases", iface);
-	nm_cmd_line_add_string (cmd, s->str);
-	g_string_truncate (s, 0);
+	nm_strv_ptrarray_add_string_printf (cmd,
+	                                    "--dhcp-leasefile=%s/dnsmasq-%s.leases",
+	                                    NMSTATEDIR,
+	                                    iface);
 
-	g_string_append (s, "--pid-file=");
-	g_string_append (s, pidfile);
-	nm_cmd_line_add_string (cmd, s->str);
-	g_string_truncate (s, 0);
+	nm_strv_ptrarray_add_string_concat (cmd, "--pid-file=", pidfile);
 
 	/* dnsmasq exits if the conf dir is not present */
 	if (g_file_test (CONFDIR, G_FILE_TEST_IS_DIR))
-		nm_cmd_line_add_string (cmd, "--conf-dir=" CONFDIR);
+		nm_strv_ptrarray_add_string_dup (cmd, "--conf-dir=" CONFDIR);
 
-	return cmd;
+	g_ptr_array_add (cmd, NULL);
+	return g_steal_pointer (&cmd);
 }
 
 static void
@@ -310,10 +265,11 @@ out:
 gboolean
 nm_dnsmasq_manager_start (NMDnsMasqManager *manager,
                           NMIP4Config *ip4_config,
+                          gboolean announce_android_metered,
                           GError **error)
 {
 	NMDnsMasqManagerPrivate *priv;
-	NMCmdLine *dm_cmd;
+	gs_unref_ptrarray GPtrArray *dm_cmd = NULL;
 	gs_free char *cmd_str = NULL;
 
 	g_return_val_if_fail (NM_IS_DNSMASQ_MANAGER (manager), FALSE);
@@ -324,32 +280,35 @@ nm_dnsmasq_manager_start (NMDnsMasqManager *manager,
 
 	kill_existing_by_pidfile (priv->pidfile);
 
-	dm_cmd = create_dm_cmd_line (priv->iface, ip4_config, priv->pidfile, error);
+	dm_cmd = create_dm_cmd_line (priv->iface,
+	                             ip4_config,
+	                             priv->pidfile,
+	                             announce_android_metered,
+	                             error);
 	if (!dm_cmd)
 		return FALSE;
 
-	g_ptr_array_add (dm_cmd->array, NULL);
-
 	_LOGI ("starting dnsmasq...");
-	_LOGD ("command line: %s", (cmd_str = nm_cmd_line_to_str (dm_cmd)));
+	_LOGD ("command line: %s", (cmd_str = g_strjoinv (" ", (char **) dm_cmd->pdata)));
 
 	priv->pid = 0;
-	if (!g_spawn_async (NULL, (char **) dm_cmd->array->pdata, NULL,
+	if (!g_spawn_async (NULL,
+	                    (char **) dm_cmd->pdata,
+	                    NULL,
 	                    G_SPAWN_DO_NOT_REAP_CHILD,
-	                    nm_utils_setpgid, NULL,
-	                    &priv->pid, error)) {
-		goto out;
-	}
+	                    nm_utils_setpgid,
+	                    NULL,
+	                    &priv->pid,
+	                    error))
+		return FALSE;
+
+	nm_assert (priv->pid > 0);
 
 	_LOGD ("dnsmasq started with pid %d", priv->pid);
 
 	priv->dm_watch_id = g_child_watch_add (priv->pid, (GChildWatchFunc) dm_watch_cb, manager);
 
- out:
-	if (dm_cmd)
-		nm_cmd_line_destroy (dm_cmd);
-
-	return priv->pid > 0;
+	return TRUE;
 }
 
 void
diff --git a/src/dnsmasq/nm-dnsmasq-manager.h b/src/dnsmasq/nm-dnsmasq-manager.h
index a0ad295cd..dd4a9069c 100644
--- a/src/dnsmasq/nm-dnsmasq-manager.h
+++ b/src/dnsmasq/nm-dnsmasq-manager.h
@@ -48,6 +48,7 @@ NMDnsMasqManager *nm_dnsmasq_manager_new (const char *iface);
 
 gboolean nm_dnsmasq_manager_start (NMDnsMasqManager *manager,
                                    NMIP4Config *ip4_config,
+                                   gboolean announce_android_metered,
                                    GError **error);
 
 void     nm_dnsmasq_manager_stop  (NMDnsMasqManager *manager);
diff --git a/src/nm-manager.c b/src/nm-manager.c
index 0710483d1..7914d16d1 100644
--- a/src/nm-manager.c
+++ b/src/nm-manager.c
@@ -1445,6 +1445,14 @@ nm_manager_update_metered (NMManager *self)
 	}
 }
 
+NMMetered
+nm_manager_get_metered (NMManager *self)
+{
+	g_return_val_if_fail (NM_IS_MANAGER (self), NM_METERED_UNKNOWN);
+
+	return NM_MANAGER_GET_PRIVATE (self)->metered;
+}
+
 static void
 nm_manager_update_state (NMManager *self)
 {
diff --git a/src/nm-manager.h b/src/nm-manager.h
index 11cba1a5c..06ca633de 100644
--- a/src/nm-manager.h
+++ b/src/nm-manager.h
@@ -174,4 +174,6 @@ void nm_manager_dbus_set_property_handle (NMDBusObject *obj,
                                           GVariant *value,
                                           gpointer user_data);
 
+NMMetered nm_manager_get_metered (NMManager *self);
+
 #endif /* __NETWORKMANAGER_MANAGER_H__ */
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