Merge pull request #15208 from apolcyn/fix_addr_sorting_compare_bug

Fix bug in an address sorting comparison

test/cpp/naming/address_sorting_test.cc

@@ -298,6 +298,29 @@ TEST(AddressSortingTest, TestUsesLabelFromDefaultTable) {
                                 });
 }
 
+/* Flip the input on the test above to reorder the sort function's
+ * comparator's inputs. */
+TEST(AddressSortingTest, TestUsesLabelFromDefaultTableInputFlipped) {
+  bool ipv4_supported = true;
+  bool ipv6_supported = true;
+  OverrideAddressSortingSourceAddrFactory(
+      ipv4_supported, ipv6_supported,
+      {
+          {"[2002::5001]:443", {"[2001::5002]:0", AF_INET6}},
+          {"[2001::5001]:443",
+           {"[2001::5002]:0", AF_INET6}},  // matching labels
+      });
+  grpc_lb_addresses* lb_addrs = BuildLbAddrInputs({
+      {"[2001::5001]:443", AF_INET6},
+      {"[2002::5001]:443", AF_INET6},
+  });
+  grpc_cares_wrapper_test_only_address_sorting_sort(lb_addrs);
+  VerifyLbAddrOutputs(lb_addrs, {
+                                    "[2001::5001]:443",
+                                    "[2002::5001]:443",
+                                });
+}
+
 /* Tests for rule 6 */
 
 TEST(AddressSortingTest,

third_party/address_sorting/address_sorting.c

@@ -225,15 +225,15 @@ static int compare_source_addr_exists(const address_sorting_sortable* first,
 static int compare_source_dest_scope_matches(
     const address_sorting_sortable* first,
     const address_sorting_sortable* second) {
-  int first_src_dst_scope_matches = 0;
+  bool first_src_dst_scope_matches = false;
   if (sockaddr_get_scope(&first->dest_addr) ==
       sockaddr_get_scope(&first->source_addr)) {
-    first_src_dst_scope_matches = 1;
+    first_src_dst_scope_matches = true;
   }
-  int second_src_dst_scope_matches = 0;
+  bool second_src_dst_scope_matches = false;
   if (sockaddr_get_scope(&second->dest_addr) ==
       sockaddr_get_scope(&second->source_addr)) {
-    second_src_dst_scope_matches = 1;
+    second_src_dst_scope_matches = true;
   }
   if (first_src_dst_scope_matches != second_src_dst_scope_matches) {
     return first_src_dst_scope_matches ? -1 : 1;
@@ -244,18 +244,18 @@ static int compare_source_dest_scope_matches(
 static int compare_source_dest_labels_match(
     const address_sorting_sortable* first,
     const address_sorting_sortable* second) {
-  int first_label_matches = 0;
+  bool first_label_matches = false;
   if (get_label_value(&first->dest_addr) ==
       get_label_value(&first->source_addr)) {
-    first_label_matches = 1;
+    first_label_matches = true;
   }
-  int second_label_matches = 0;
+  bool second_label_matches = false;
   if (get_label_value(&second->dest_addr) ==
       get_label_value(&second->source_addr)) {
-    second_label_matches = 1;
+    second_label_matches = true;
   }
   if (first_label_matches != second_label_matches) {
-    return first_label_matches ? 1 : 1;
+    return first_label_matches ? -1 : 1;
   }
   return 0;
 }