+        if (--nmc->should_wait == 0) {
+         if (nmc->return_value == NMC_RESULT_ERROR_NOT_FOUND) {
+              g_string_printf (nmc->return_text, _("Error: Access point with bssid '%s' not found."),
+                               data->bssid_user);
+         }
+                g_main_loop_quit (loop);
+    }

intention




+wifi_list_rescan_cb (GObject *source_object, GAsyncResult *res, gpointer user_data)
+{
+    NMDeviceWifi *wifi = NM_DEVICE_WIFI (source_object);
+    WifiListData *data = user_data;
+    gs_free_error GError *error = NULL;
+
+    if (!nm_device_wifi_request_scan_finish (wifi, res, &error)) {
+         if (   g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)
+             || g_error_matches (error, NM_DEVICE_ERROR, NM_DEVICE_ERROR_NOT_ALLOWED))
+              return;

why does it not do anything in case NM_DEVICE_ERROR_NOT_ALLOWED?
Also, in the cancelled case, will wifi_list_finish() ever be called to cleanup the data?


time_t rescan_cutoff

The choice of the D-Bus type for LastScan and nm_device_wifi_get_last_scan() already makes it clear that the program is not gonna work with anything outside the range of int32. Let's not compare or do arithmetic with intergers of different (even unknown) types like time_t. Just convert time_t early on to int/gint32 and use a consistent integer type.




> cli/devices: parse "nmcli d wifi list" arguments with next_arg()

-         if (strcmp (*argv, "ifname") == 0) {
+         case 1: /* ifname */

is this really better then before? Instead of explicitly having the option name, it's now a number with a comment.

Ok, whatever. I don't really mind. But there is also matches_arg() which has an odd overlap with argument parsing in next_arg(). Especially, matches_arg() can support forms like --rescan=auto.
Also, next_arg() explicitly completes
   nmc_complete_strings (**argv, "--ask", "--show-secrets", NULL);
which is wrong for most subcommands

  $ nmcli --complete-args networking connectivity -
  -help
  --help
  --ask
  --show-secrets

TODO.


regardless of how fres the access point list is.</para>
                  ^^^^




-    if (ifname) {
+    if (rescan == NULL || strcmp (rescan, "auto") == 0) {
+         struct timespec tp;
 
+         if (clock_gettime (CLOCK_BOOTTIME, &tp) == -1) {
+              g_printerr (_("Failed to get CLOCK_BOOTTIME: %s\n"), strerror (errno));
+              rescan_cutoff = 0;
+         } else {
+              rescan_cutoff = MAX (tp.tv_sec - 30, 0);
+         }
+    } else if (strcmp (rescan, "no") == 0) {
+         rescan_cutoff = 0;
+    } else if (strcmp (rescan, "yes") == 0) {
+         rescan_cutoff = -1;
+    } else {
+         g_string_printf (nmc->return_text, _("Error: invalid rescan argument: '%s' not among [auto, no, yes]"), rescan);
+         return NMC_RESULT_ERROR_USER_INPUT;

optimally, the options would be validated left to right. While certain validation is not possible right away, just validating for auto|no|yes is possible right away. That should be done inside the loop above.

Maybe expose the timestamp with milliseconds precision? (and type gint64).

I know, AccessPoint's LastSeen is in the same scale as you added LastScan, which is good for consistency. On the other hand, CheckPoint's "Created" timestamp is also in milliseconds.

Milliseconds precision is plenty for LastScan, whole seconds barely.

Thanks for the review. Following up here:

https://github.com/NetworkManager/NetworkManager/pull/128