From ca4b5307426bc93e4df03aca62fc8182de27765b Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 9 Mar 2020 17:08:08 +0100 Subject: [PATCH] cli: use NM_CMP*() macros for compare_aps() in "clients/cli/devices.c" The compare pattern seems simple, but seems error prone and subtle. NM_CMP*() avoids that. For example, nm_access_point_get_strength() returns an uint8_t. C will promote those values to "int" before doing the subtraction. Likewise, nm_access_point_get_frequency() returns a uint32_t. This gets promoted to unsigned int when doing the subtraction. Afterwards, that is converted to a signed int. So both cases were in fact correct. But such things are not obvious. Also, as fallback sort by D-Bus path. While that is not semantically useful, we should use a defined sort order. --- clients/cli/devices.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/clients/cli/devices.c b/clients/cli/devices.c index 964b7dce83..302ab0f18a 100644 --- a/clients/cli/devices.c +++ b/clients/cli/devices.c @@ -1101,17 +1101,16 @@ 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 *