[Asterisk-code-review] menuselect: Resolve infinite loop in dependency scenario. (asterisk[17])

[Friendly Automation]

Friendly Automation has submitted this change. ( https://gerrit.asterisk.org/c/asterisk/+/14608 )

Change subject: menuselect: Resolve infinite loop in dependency scenario.
......................................................................

menuselect: Resolve infinite loop in dependency scenario.

Given a scenario where a module has a dependency on both
an external library and a module if the external library was
available and the module was not an infinite loop would
occur. This happened due to the code changing the dependecy
status to no failure on each dependency checking loop
iteration, resulting in the code thinking that it had
gone from no failure to failure each time triggering another
dependency check.

This change makes it so that the old dependency status is
preserved throughout the dependency checking allowing it to
determine that after the first iteration the dependency
status does not transition from no failure to failure.

ASTERISK-28930

Change-Id: Iea06d45d9fd6d8bfd068882a0bb7e23a53ec3e84
---
M menuselect/menuselect.c
M menuselect/menuselect.h
2 files changed, 8 insertions(+), 6 deletions(-)

Approvals:
  Kevin Harwell: Looks good to me, approved
  George Joseph: Looks good to me, but someone else must approve
  Friendly Automation: Approved for Submit



diff --git a/menuselect/menuselect.c b/menuselect/menuselect.c
index 908b2ea..2eea3f0 100644
--- a/menuselect/menuselect.c
+++ b/menuselect/menuselect.c
@@ -630,14 +630,14 @@
 	struct member *mem;
 	struct reference *dep;
 	struct dep_file *dep_file;
-	unsigned int changed, old_failure;
+	unsigned int changed;
 
 	AST_LIST_TRAVERSE(&categories, cat, list) {
 		AST_LIST_TRAVERSE(&cat->members, mem, list) {
 			if (mem->is_separator) {
 				continue;
 			}
-			old_failure = mem->depsfailed;
+			mem->depsfailedold = mem->depsfailed;
 			AST_LIST_TRAVERSE(&mem->deps, dep, list) {
 				if (dep->member)
 					continue;
@@ -655,7 +655,7 @@
 					break; /* This dependency is not met, so we can stop now */
 				}
 			}
-			if (old_failure == SOFT_FAILURE && mem->depsfailed != HARD_FAILURE)
+			if (mem->depsfailedold == SOFT_FAILURE && mem->depsfailed != HARD_FAILURE)
 				mem->depsfailed = SOFT_FAILURE;
 		}
 	}
@@ -673,8 +673,6 @@
 					continue;
 				}
 
-				old_failure = mem->depsfailed;
-
 				if (mem->depsfailed == HARD_FAILURE)
 					continue;
 
@@ -693,7 +691,7 @@
 					}
 				}
 
-				if (mem->depsfailed != old_failure) {
+				if (mem->depsfailed != mem->depsfailedold) {
 					if ((mem->depsfailed == NO_FAILURE) && mem->was_defaulted) {
 						mem->enabled = !strcasecmp(mem->defaultenabled, "yes");
 						print_debug("Just set %s enabled to %d\n", mem->name, mem->enabled);
@@ -702,6 +700,8 @@
 						print_debug("Just set %s enabled to %d\n", mem->name, mem->enabled);
 					}
 					changed = 1;
+					/* We need to update the old failed deps for the next loop of this */
+					mem->depsfailedold = mem->depsfailed;
 					break; /* This dependency is not met, so we can stop now */
 				}
 			}
diff --git a/menuselect/menuselect.h b/menuselect/menuselect.h
index 78ae8ef..d41859e 100644
--- a/menuselect/menuselect.h
+++ b/menuselect/menuselect.h
@@ -78,6 +78,8 @@
 	unsigned int was_enabled:1;
 	/*! This module has failed dependencies */
 	unsigned int depsfailed:2;
+	/*! Previous failed dependencies when calculating */
+	unsigned int depsfailedold:2;
 	/*! This module has failed conflicts */
 	unsigned int conflictsfailed:2;
 	/*! This module's 'enabled' flag was changed by a default only */

-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/14608
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 17
Gerrit-Change-Id: Iea06d45d9fd6d8bfd068882a0bb7e23a53ec3e84
Gerrit-Change-Number: 14608
Gerrit-PatchSet: 2
Gerrit-Owner: Joshua Colp <jcolp at sangoma.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-CC: Jaco Kroon <jaco at uls.co.za>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20200625/da98b717/attachment-0001.html>