[Asterisk-code-review] dns srv: Fix SRV sorting when records with priority zero exi... (asterisk[master])

[Joshua Colp]

Joshua Colp has submitted this change and it was merged.

Change subject: dns_srv: Fix SRV sorting when records with priority zero exist with non-zero.
......................................................................


dns_srv: Fix SRV sorting when records with priority zero exist with non-zero.

The DNS SRV sorting code currently has an issue when records with a priority
of zero exist with records of a non-zero priority. This occurs because the
sorting code considers zero to mean unset when in reality is a valid
value. If the current priority is zero it will get replaced with any remaining
record that has a priority of non-zero, until no records of those exist after
which the records of priority zero are handled.

This change makes it so that the priority of the first remaining record is
the current starting priority. There is also a small optimization to prevent
iterating records when the starting priority is already zero.

Change-Id: I103511f35b50428f770bd4db3ffef70fb6f82d35
---
M main/dns_srv.c
1 file changed, 7 insertions(+), 5 deletions(-)

Approvals:
  Kevin Harwell: Looks good to me, but someone else must approve
  Joshua Colp: Looks good to me, approved; Verified



diff --git a/main/dns_srv.c b/main/dns_srv.c
index f5d038a..e4a3d8b 100644
--- a/main/dns_srv.c
+++ b/main/dns_srv.c
@@ -112,13 +112,15 @@
 	struct dns_records newlist = AST_LIST_HEAD_NOLOCK_INIT_VALUE;
 
 	while (AST_LIST_FIRST(&result->records)) {
-		unsigned short cur_priority = 0;
+		unsigned short cur_priority = ((struct ast_dns_srv_record *)(AST_LIST_FIRST(&result->records)))->priority;
 		struct dns_records temp_list = AST_LIST_HEAD_NOLOCK_INIT_VALUE;
 
-		/* Find the lowest current priority to work on */
-		AST_LIST_TRAVERSE(&result->records, current, list) {
-			if (!cur_priority || ((struct ast_dns_srv_record *)current)->priority < cur_priority) {
-				cur_priority = ((struct ast_dns_srv_record *)current)->priority;
+		/* Find the lowest current priority to work on, but if the priority is already zero there is no lower priority */
+		if (cur_priority) {
+			AST_LIST_TRAVERSE(&result->records, current, list) {
+				if (((struct ast_dns_srv_record *)current)->priority < cur_priority) {
+					cur_priority = ((struct ast_dns_srv_record *)current)->priority;
+				}
 			}
 		}
 

-- 
To view, visit https://gerrit.asterisk.org/429
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I103511f35b50428f770bd4db3ffef70fb6f82d35
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>