cli: merge branch 'th/cli-device-wifi-cleanup'

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/435

2 files changed, 125 insertions, 90 deletions

diff --git a/clients/cli/devices.c b/clients/cli/devices.c
index 964b7dce83..464cbf3eb7 100644
--- a/clients/cli/devices.c
+++ b/clients/cli/devices.c
@@ -27,9 +27,9 @@
 /*****************************************************************************/
 
 static char *
-ap_wpa_rsn_flags_to_string (NM80211ApSecurityFlags flags)
+ap_wpa_rsn_flags_to_string (NM80211ApSecurityFlags flags, NMMetaAccessorGetType get_type)
 {
-	char *flags_str[14];
+	char *flags_str[16];
 	int i = 0;
 
 	if (flags & NM_802_11_AP_SEC_PAIR_WEP40)
@@ -56,13 +56,17 @@ ap_wpa_rsn_flags_to_string (NM80211ApSecurityFlags flags)
 		flags_str[i++] = "sae";
 	if (flags & NM_802_11_AP_SEC_KEY_MGMT_OWE)
 		flags_str[i++] = "owe";
+
 	/* Make sure you grow flags_str when adding items here. */
+	nm_assert (i < G_N_ELEMENTS (flags_str));
 
-	if (i == 0)
-		flags_str[i++] = _("(none)");
+	if (i == 0) {
+		if (get_type == NM_META_ACCESSOR_GET_TYPE_PRETTY)
+			return g_strdup (_("(none)"));
+		return g_strdup ("(none)");
+	}
 
 	flags_str[i] = NULL;
-
 	return g_strjoinv (" ", flags_str);
 }
 
@@ -1101,30 +1105,30 @@ compare_aps (gconstpointer a, gconstpointer b, gpointer user_data)
 {
 	NMAccessPoint *apa = *(NMAccessPoint **)a;
 	NMAccessPoint *apb = *(NMAccessPoint **)b;
-	int cmp;
 
-	cmp = nm_access_point_get_strength (apb) - nm_access_point_get_strength (apa);
-	if (cmp != 0)
-		return cmp;
+	NM_CMP_DIRECT (nm_access_point_get_strength (apb), nm_access_point_get_strength (apa));
+	NM_CMP_DIRECT (nm_access_point_get_frequency (apa), nm_access_point_get_frequency (apb));
+	NM_CMP_DIRECT (nm_access_point_get_max_bitrate (apb), nm_access_point_get_max_bitrate (apa));
 
-	cmp = nm_access_point_get_frequency (apa) - nm_access_point_get_frequency (apb);
-	if (cmp != 0)
-		return cmp;
+	/* as fallback, just give it some stable order and use the D-Bus path (literally). */
+	NM_CMP_DIRECT_STRCMP0 (nm_object_get_path (NM_OBJECT (apa)),
+	                       nm_object_get_path (NM_OBJECT (apb)));
 
-	return nm_access_point_get_max_bitrate (apb) - nm_access_point_get_max_bitrate (apa);
+	return 0;
 }
 
 static GPtrArray *
 sort_access_points (const GPtrArray *aps)
 {
 	GPtrArray *sorted;
-	int i;
+	guint i;
 
 	g_return_val_if_fail (aps, NULL);
 
 	sorted = g_ptr_array_sized_new (aps->len);
+	g_ptr_array_set_free_func (sorted, nm_g_object_unref);
 	for (i = 0; i < aps->len; i++)
-		g_ptr_array_add (sorted, aps->pdata[i]);
+		g_ptr_array_add (sorted, g_object_ref (aps->pdata[i]));
 	g_ptr_array_sort_with_data (sorted, compare_aps, NULL);
 	return sorted;
 }
@@ -1189,8 +1193,8 @@ fill_output_access_point (gpointer data, gpointer user_data)
 	freq_str = g_strdup_printf (_("%u MHz"), freq);
 	bitrate_str = g_strdup_printf (_("%u Mbit/s"), bitrate/1000);
 	strength_str = g_strdup_printf ("%u", strength);
-	wpa_flags_str = ap_wpa_rsn_flags_to_string (wpa_flags);
-	rsn_flags_str = ap_wpa_rsn_flags_to_string (rsn_flags);
+	wpa_flags_str = ap_wpa_rsn_flags_to_string (wpa_flags, NM_META_ACCESSOR_GET_TYPE_PRETTY);
+	rsn_flags_str = ap_wpa_rsn_flags_to_string (rsn_flags, NM_META_ACCESSOR_GET_TYPE_PRETTY);
 	sig_bars = nmc_wifi_strength_bars (strength);
 
 	security_str = g_string_new (NULL);
@@ -1543,7 +1547,6 @@ show_device_info (NMDevice *device, NmCli *nmc)
 		if ((NM_IS_DEVICE_WIFI (device))) {
 			NMAccessPoint *active_ap = NULL;
 			const char *active_bssid = NULL;
-			GPtrArray *aps;
 
 			/* section AP */
 			if (!g_ascii_strcasecmp (nmc_fields_dev_show_sections[section_idx]->name, nmc_fields_dev_show_sections[4]->name)) {
@@ -1561,6 +1564,7 @@ show_device_info (NMDevice *device, NmCli *nmc)
 				g_ptr_array_add (out.output_data, arr);
 
 				{
+					gs_unref_ptrarray GPtrArray *aps = NULL;
 					APInfo info = {
 						.nmc = nmc,
 						.index = 1,
@@ -1572,7 +1576,6 @@ show_device_info (NMDevice *device, NmCli *nmc)
 
 					aps = sort_access_points (nm_device_wifi_get_access_points (NM_DEVICE_WIFI (device)));
 					g_ptr_array_foreach (aps, fill_output_access_point, &info);
-					g_ptr_array_free (aps, FALSE);
 				}
 
 				print_data_prepare_width (out.output_data);
@@ -2785,7 +2788,6 @@ show_access_point_info (NMDeviceWifi *wifi, NmCli *nmc, NmcOutputData *out)
 {
 	NMAccessPoint *active_ap = NULL;
 	const char *active_bssid = NULL;
-	GPtrArray *aps;
 	NmcOutputField *arr;
 
 	if (nm_device_get_state (NM_DEVICE (wifi)) == NM_DEVICE_STATE_ACTIVATED) {
@@ -2798,6 +2800,7 @@ show_access_point_info (NMDeviceWifi *wifi, NmCli *nmc, NmcOutputData *out)
 	g_ptr_array_add (out->output_data, arr);
 
 	{
+		gs_unref_ptrarray GPtrArray *aps = NULL;
 		APInfo info = {
 			.nmc = nmc,
 			.index = 1,
@@ -2809,7 +2812,6 @@ show_access_point_info (NMDeviceWifi *wifi, NmCli *nmc, NmcOutputData *out)
 
 		aps = sort_access_points (nm_device_wifi_get_access_points (wifi));
 		g_ptr_array_foreach (aps, fill_output_access_point, &info);
-		g_ptr_array_free (aps, TRUE);
 	}
 
 	print_data_prepare_width (out->output_data);
@@ -2923,14 +2925,14 @@ wifi_list_finish (WifiListData *data)
 	g_signal_handler_disconnect (data->wifi, data->last_scan_id);
 	nm_clear_g_source (&data->timeout_id);
 	nm_clear_g_cancellable (&data->scan_cancellable);
-	g_slice_free (WifiListData, data);
+	nm_g_slice_free (data);
 
 	if (info->nmc->should_wait == 0) {
 		for (i = 0; info->devices[i]; i++)
 			g_object_unref (info->devices[i]);
 		g_free (info->devices);
 		g_array_unref (info->out_indices);
-		g_free (info);
+		nm_g_slice_free (info);
 	}
 }
 
@@ -3012,6 +3014,8 @@ do_device_wifi_list (NmCli *nmc, int argc, char **argv)
 	NMDeviceWifi *wifi;
 	ScanInfo *scan_info = NULL;
 	WifiListData *data;
+	gboolean ifname_handled;
+	NMDevice *ifname_handled_candidate;
 	guint i, j;
 
 	devices = nmc_get_devices_sorted (nmc->client);
@@ -3060,6 +3064,9 @@ do_device_wifi_list (NmCli *nmc, int argc, char **argv)
 		}
 	}
 
+	if (nmc->complete)
+		return nmc->return_value;
+
 	if (!nmc->required_fields || g_ascii_strcasecmp (nmc->required_fields, "common") == 0)
 		fields_str = NMC_FIELDS_DEV_WIFI_LIST_COMMON;
 	else if (!nmc->required_fields || g_ascii_strcasecmp (nmc->required_fields, "all") == 0) {
@@ -3075,9 +3082,6 @@ do_device_wifi_list (NmCli *nmc, int argc, char **argv)
 		return NMC_RESULT_ERROR_USER_INPUT;
 	}
 
-	if (nmc->complete)
-		return nmc->return_value;
-
 	if (argc) {
 		g_string_printf (nmc->return_text, _("Error: invalid extra argument '%s'."), *argv);
 		return NMC_RESULT_ERROR_USER_INPUT;
@@ -3094,63 +3098,91 @@ do_device_wifi_list (NmCli *nmc, int argc, char **argv)
 		return NMC_RESULT_ERROR_USER_INPUT;
 	}
 
-	if (ifname) {
-		device = find_wifi_device_by_iface (devices, ifname, NULL);
-		if (!device) {
-			g_string_printf (nmc->return_text, _("Error: Device '%s' not found."), ifname);
-			return NMC_RESULT_ERROR_NOT_FOUND;
-		}
+	ifname_handled = (ifname == NULL);
+	ifname_handled_candidate = NULL;
 
-		if (NM_IS_DEVICE_WIFI (device)) {
-			devices[0] = device;
-			devices[1] = NULL;
-		} else {
-			if (   nm_device_get_device_type (device) == NM_DEVICE_TYPE_GENERIC
-			    && g_strcmp0 (nm_device_get_type_description (device), "wifi") == 0) {
-				g_string_printf (nmc->return_text,
-				                 _("Error: Device '%s' was not recognized as a Wi-Fi device, check NetworkManager Wi-Fi plugin."),
-				                 ifname);
-			} else {
-				g_string_printf (nmc->return_text,
-				                 _("Error: Device '%s' is not a Wi-Fi device."),
-				                 ifname);
+	j = 0;
+	for (i = 0; devices[i]; i++) {
+		const char *dev_iface;
+
+		device = devices[i];
+		dev_iface = nm_device_get_iface (device);
+
+		if (ifname) {
+			if (!nm_streq0 (ifname, dev_iface))
+				continue;
+			if (!NM_IS_DEVICE_WIFI (device)) {
+				if (   nm_device_get_device_type (device) == NM_DEVICE_TYPE_GENERIC
+				    && nm_streq0 (nm_device_get_type_description (device), "wifi"))
+					ifname_handled_candidate = device;
+				else if (!ifname_handled_candidate)
+					ifname_handled_candidate = device;
+				continue;
 			}
-			return NMC_RESULT_ERROR_UNKNOWN;
+			ifname_handled = TRUE;
+		} else {
+			if (!NM_IS_DEVICE_WIFI (device))
+				continue;
 		}
-	}
 
-	/* Filter out non-wifi devices */
-	for (i = 0, j = 0; devices[i]; i++) {
-		if (NM_IS_DEVICE_WIFI (devices[i]))
-			devices[j++] = devices[i];
+		devices[j++] = device;
 	}
 	devices[j] = NULL;
 
+	if (!ifname_handled) {
+		if (!ifname_handled_candidate) {
+			g_string_printf (nmc->return_text,
+			                 _("Error: Device '%s' not found."),
+			                 ifname);
+		} else if (   nm_device_get_device_type (ifname_handled_candidate) == NM_DEVICE_TYPE_GENERIC
+		           && nm_streq0 (nm_device_get_type_description (ifname_handled_candidate), "wifi")) {
+			g_string_printf (nmc->return_text,
+			                 _("Error: Device '%s' was not recognized as a Wi-Fi device, check NetworkManager Wi-Fi plugin."),
+			                 ifname);
+		} else {
+			g_string_printf (nmc->return_text,
+			                 _("Error: Device '%s' is not a Wi-Fi device."),
+			                 ifname);
+		}
+		return NMC_RESULT_ERROR_NOT_FOUND;
+	}
+
 	/* Start a new scan for devices that need it */
 	for (i = 0; devices[i]; i++) {
 		wifi = (NMDeviceWifi *) devices[i];
 		g_object_ref (wifi);
 
 		if (   rescan_cutoff == 0
-		    || (rescan_cutoff > 0 && nm_device_wifi_get_last_scan (wifi) >= rescan_cutoff))
+		    || (   rescan_cutoff > 0
+		        && nm_device_wifi_get_last_scan (wifi) >= rescan_cutoff))
 			continue;
 
 		if (!scan_info) {
-			scan_info = g_new0 (ScanInfo, 1);
-			scan_info->out_indices = g_array_ref (out_indices);
-			scan_info->tmpl = tmpl;
-			scan_info->bssid_user = bssid_user;
-			scan_info->nmc = nmc;
+			scan_info = g_slice_new (ScanInfo);
+			*scan_info = (ScanInfo) {
+				.out_indices = g_array_ref (out_indices),
+				.tmpl        = tmpl,
+				.bssid_user  = bssid_user,
+				.nmc         = nmc,
+			};
 		}
 
 		nmc->should_wait++;
-		data = g_slice_new0 (WifiListData);
-		data->wifi = wifi;
-		data->scan_info = scan_info;
-		data->last_scan_id = g_signal_connect (wifi, "notify::" NM_DEVICE_WIFI_LAST_SCAN,
-		                                       G_CALLBACK (wifi_last_scan_updated), data);
-		data->scan_cancellable = g_cancellable_new ();
-		data->timeout_id = g_timeout_add_seconds (15, wifi_list_scan_timeout, data);
+
+		data = g_slice_new (WifiListData);
+		*data = (WifiListData) {
+			.wifi             = wifi,
+			.scan_info        = scan_info,
+			.last_scan_id     = g_signal_connect (wifi,
+			                                      "notify::" NM_DEVICE_WIFI_LAST_SCAN,
+			                                      G_CALLBACK (wifi_last_scan_updated),
+			                                      data),
+			.scan_cancellable = g_cancellable_new (),
+			.timeout_id       = g_timeout_add_seconds (15,
+			                                           wifi_list_scan_timeout,
+			                                           data),
+		};
+
 		nm_device_wifi_request_scan_async (wifi, data->scan_cancellable, wifi_list_rescan_cb, data);
 	}
 
diff --git a/clients/cli/utils.c b/clients/cli/utils.c
index 2e3a38f735..d8524c55a1 100644
--- a/clients/cli/utils.c
+++ b/clients/cli/utils.c
@@ -321,19 +321,14 @@ nmc_parse_args (nmc_arg_t *arg_arr, gboolean last, int *argc, char ***argv, GErr
 char *
 ssid_to_hex (const char *str, gsize len)
 {
-	GString *printable;
-	char *printable_str;
-	int i;
-
-	if (str == NULL || len == 0)
+	if (len == 0)
 		return NULL;
 
-	printable = g_string_new (NULL);
-	for (i = 0; i < len; i++) {
-		g_string_append_printf (printable, "%02X", (unsigned char) str[i]);
-	}
-	printable_str = g_string_free (printable, FALSE);
-	return printable_str;
+	return nm_utils_bin2hexstr_full (str,
+	                                 len,
+	                                 '\0',
+	                                 TRUE,
+	                                 NULL);
 }
 
 /*
@@ -667,7 +662,6 @@ _output_selection_append (GArray *cols,
 	guint i;
 	const NMMetaAbstractInfo *const*nested;
 	NMMetaSelectionResultList *selection;
-	const NMMetaSelectionItem *si;
 
 	col_idx = cols->len;
 
@@ -688,6 +682,8 @@ _output_selection_append (GArray *cols,
 			gs_free char *allowed_fields = NULL;
 
 			if (parent_idx != PRINT_DATA_COL_PARENT_NIL) {
+				const NMMetaSelectionItem *si;
+
 				si = g_array_index (cols, PrintDataCol, parent_idx).selection_item;
 				allowed_fields = nm_meta_abstract_info_get_nested_names_str (si->info, si->self_selection);
 			}
@@ -719,10 +715,9 @@ _output_selection_append (GArray *cols,
 		g_ptr_array_add (gfree_keeper, selection);
 
 		for (i = 0; i < selection->num; i++) {
-			si = &selection->items[i];
 			if (!_output_selection_append (cols,
 			                               col_idx,
-			                               si,
+			                               &selection->items[i],
 			                               gfree_keeper,
 			                               error))
 				return FALSE;
@@ -778,7 +773,8 @@ _output_selection_complete (GArray *cols)
 static gboolean
 _output_selection_parse (const NMMetaAbstractInfo *const*fields,
                          const char *fields_str,
-                         GArray **out_cols,
+                         PrintDataCol **out_cols_data,
+                         guint *out_cols_len,
                          GPtrArray **out_gfree_keeper,
                          GError **error)
 {
@@ -803,16 +799,18 @@ _output_selection_parse (const NMMetaAbstractInfo *const*fields,
 	cols = g_array_new (FALSE, TRUE, sizeof (PrintDataCol));
 
 	for (i = 0; i < selection->num; i++) {
-		const NMMetaSelectionItem *si = &selection->items[i];
-
-		if (!_output_selection_append (cols, PRINT_DATA_COL_PARENT_NIL,
-		                               si, gfree_keeper, error))
+		if (!_output_selection_append (cols,
+		                               PRINT_DATA_COL_PARENT_NIL,
+		                               &selection->items[i],
+		                               gfree_keeper,
+		                               error))
 			return FALSE;
 	}
 
 	_output_selection_complete (cols);
 
-	*out_cols = g_steal_pointer (&cols);
+	*out_cols_len = cols->len;
+	*out_cols_data = (PrintDataCol *) g_array_free (g_steal_pointer (&cols), FALSE);
 	*out_gfree_keeper = g_steal_pointer (&gfree_keeper);
 	return TRUE;
 }
@@ -1142,7 +1140,8 @@ _print_fill (const NmcConfig *nmc_config,
 		header_cell->width = nmc_string_screen_width (header_cell->title, NULL);
 
 		for (i_row = 0; i_row < targets_len; i_row++) {
-			const PrintDataCell *cell = &g_array_index (cells, PrintDataCell, i_row * cols_len + i_col);
+			const PrintDataCell *cells_line = &g_array_index (cells, PrintDataCell, i_row * header_row->len);
+			const PrintDataCell *cell = &cells_line[i_col];
 			const char *const*i_strv;
 
 			switch (cell->text_format) {
@@ -1379,20 +1378,24 @@ nmc_print (const NmcConfig *nmc_config,
            GError **error)
 {
 	gs_unref_ptrarray GPtrArray *gfree_keeper = NULL;
-	gs_unref_array GArray *cols = NULL;
+	gs_free PrintDataCol *cols_data = NULL;
+	guint cols_len;
 	gs_unref_array GArray *header_row = NULL;
 	gs_unref_array GArray *cells = NULL;
 
-	if (!_output_selection_parse (fields, fields_str,
-	                              &cols, &gfree_keeper,
+	if (!_output_selection_parse (fields,
+	                              fields_str,
+	                              &cols_data,
+	                              &cols_len,
+	                              &gfree_keeper,
 	                              error))
 		return FALSE;
 
 	_print_fill (nmc_config,
 	             targets,
 	             targets_data,
-	             &g_array_index (cols, PrintDataCol, 0),
-	             cols->len,
+	             cols_data,
+	             cols_len,
 	             &header_row,
 	             &cells);