From 0d98c8d9f0bee1c4f2b9a58955b1659ecc0a2bdb Mon Sep 17 00:00:00 2001 From: Alexander Polcyn Date: Thu, 26 Apr 2018 20:51:50 -0700 Subject: [PATCH 1/2] Fix a bug in an address sorting comparison --- test/cpp/naming/address_sorting_test.cc | 23 +++++++++++++++++++ third_party/address_sorting/address_sorting.c | 2 +- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/test/cpp/naming/address_sorting_test.cc b/test/cpp/naming/address_sorting_test.cc index a423733cafa..a92e9e3b3e3 100644 --- a/test/cpp/naming/address_sorting_test.cc +++ b/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, diff --git a/third_party/address_sorting/address_sorting.c b/third_party/address_sorting/address_sorting.c index d62aca34246..6aa1f994e38 100644 --- a/third_party/address_sorting/address_sorting.c +++ b/third_party/address_sorting/address_sorting.c @@ -255,7 +255,7 @@ static int compare_source_dest_labels_match( second_label_matches = 1; } if (first_label_matches != second_label_matches) { - return first_label_matches ? 1 : 1; + return first_label_matches ? -1 : 1; } return 0; } From 7e1687b3ef79d8213cff6a0df66d33b01013c9ad Mon Sep 17 00:00:00 2001 From: Alexander Polcyn Date: Fri, 27 Apr 2018 18:30:12 -0700 Subject: [PATCH 2/2] address comment --- third_party/address_sorting/address_sorting.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/third_party/address_sorting/address_sorting.c b/third_party/address_sorting/address_sorting.c index 6aa1f994e38..e4f3b537992 100644 --- a/third_party/address_sorting/address_sorting.c +++ b/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,15 +244,15 @@ 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;