[PATCH] Fix comparison of services of same preferred type

The service_compare_preferred() function is called twice in
service_compare(), once with a guard to only use it on services with
differing types, but once without any such guard. In the latter case,
when comparing two services of the same preferred technology, it always
returns -1 because it checks service_a first and exits immediately, when
one would normally expect that comparison to result in a tie (return
value 0).

This causes the parent service_compare() function to terminate its
comparison early instead of applying appropriate tie-breakers like
online status, Strength or Name. The result is an unstable sort order
that depends on the initial order of services.

This patch adds a condition to service_compare_preferred() so that
the preference logic is only applied to services of different types,
without relying on calling contexts to make such checks.

I ran into this issue when reading sources to understand how ConnMan
would sort two connected Ethernet services, when one is 'online' and one
is 'ready' and 'ethernet' is in the preferred technologies list. It
seems that their online state would in fact not matter at all, instead
the service that happened to appear first in the list would win.

The issue may have been introduced in
31d9ce64a06743cc69b92c47a36e5ace0f9cb030, when
service_compare_preferred() was extracted from its original context in
which the comparison was only applied to services of unequal types.

Signed-off-by: Johannes Emerich <[email protected]>
---
 src/service.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/service.c b/src/service.c
index f73f42eb..d5869b14 100644
--- a/src/service.c
+++ b/src/service.c
@@ -8299,9 +8299,9 @@ static gint service_compare_vpn(const struct connman_service *a,
  *                         to compare by its technology type with the
  *                         @a PreferredTechnologies priority list.
  *
- *  @retval   0  If the @a PreferredTechnologies configuration is empty
- *               or if neither service type matches a technology type
- *               in the @a PreferredTechnologies list.
+ *  @retval   0  If neither service type appears in the
+ *               @a PreferredTechnologies list (including when
+ *               it is empty), or if the two service types are equal.
  *  @retval  -1  If @a service_a type matches a technology type
  *               in the @a PreferredTechnologies list and should sort
  *               @b before @a service_b.
@@ -8317,7 +8317,7 @@ static gint service_compare_preferred(const struct connman_service *service_a,
 	int i;
 
 	tech_array = connman_setting_get_uint_list("PreferredTechnologies");
-	if (tech_array) {
+	if (tech_array && service_a->type != service_b->type) {
 		for (i = 0; tech_array[i]; i++) {
 			if (tech_array[i] == service_a->type)
 				return -1;
-- 
2.47.2