[syslinux] [GIT PULL] elflink core

[Matt Fleming]

Hi,

I recently ran into an issue where the dependencies generated in
modules.dep created a circular reference, which meant that qemu spun
forever in modules_load_dependencies().

I did think about detecting this kind of circular dependency at
modules.dep generation time but if we ever move to using DT_NEEDED to
track dependencies we would need this functionality in the elflink core
anyway. Besides, it's not that much code.

The following changes since commit 8c576f1fe03e34879921311f46613a35c6530000:

  Merge remote-tracking branch 'mfleming/for-hpa/elflink/fix-compiler-warnings' into elflink (2011-03-16 12:53:58 -0700)

are available in the git repository at:

  git://git.zytor.com/users/mfleming/syslinux.git for-hpa/elflink/core

Matt Fleming (1):
      elflink: Detect circular dependencies during module load

 com32/lib/sys/module/exec.c |   84 ++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 80 insertions(+), 4 deletions(-)

diff --git a/com32/lib/sys/module/exec.c b/com32/lib/sys/module/exec.c
index 54182cc..fbe165d 100644
--- a/com32/lib/sys/module/exec.c
+++ b/com32/lib/sys/module/exec.c
@@ -343,11 +343,45 @@ int spawn_load(const char *name,const char **argv)
 	*/
 }
 
+/*
+ * Avoid circular dependencies.
+ *
+ * It's possible that someone messed up the modules.dep file and that
+ * it includes circular dependencies, so we need to take steps here to
+ * avoid looping in module_load_dependencies() forever.
+ *
+ * We build a singly-linked list of modules that are in the middle of
+ * being loaded. When they have completed loading their entry is
+ * removed from this list in LIFO order (new entries are always added
+ * to the head of the list).
+ */
+struct loading_dep {
+	const char *name;
+	struct module_dep *next;
+};
+static struct loading_dep *loading_deps;
+
+/*
+ * Remember that because we insert elements in a LIFO order we need to
+ * start from the end of the list and work towards the front so that
+ * we print the modules in the order in which we tried to load them.
+ *
+ * Yay for recursive function calls.
+ */
+static void print_loading_dep(struct loading_dep *dep)
+{
+	if (dep) {
+		print_loading_dep(dep->next);
+		printf("\t\t\"%s\"\n", dep->name);
+	}
+}
+
 int module_load_dependencies(const char *name,const char *dep_file)
 {
 	FILE *d_file=fopen(dep_file,"r");
 	char line[2048],aux[2048],temp_name[MODULE_NAME_SIZE],slbz[24];
 	int i=0,j=0,res=0;
+	struct loading_dep *dep;
 
 	if(d_file==NULL)
 	{
@@ -355,6 +389,40 @@ int module_load_dependencies(const char *name,const char *dep_file)
 		return -1;
 	}
 
+	/*
+	 * Are we already in the middle of loading this module's
+	 * dependencies?
+	 */
+	for (dep = loading_deps; dep; dep = dep->next) {
+		if (!strcasecmp(dep->name, name))
+			break;	/* found */
+	}
+
+	if (dep) {
+		struct loading_dep *last, *prev;
+
+		/* Dup! */
+		printf("\t\tCircular depedency detected when loading "
+		       "modules!\n");
+		printf("\t\tModules dependency chain looks like this,\n\n");
+		print_loading_dep(loading_deps);
+		printf("\n\t\t... and we tried to load \"%s\" again\n", name);
+
+		return -1;
+	} else {
+		dep = malloc(sizeof(*dep));
+		if (!dep) {
+			printf("Failed to alloc memory for loading_dep\n");
+			return -1;
+		}
+		
+		dep->name = name;
+		dep->next = loading_deps;
+
+		/* Insert at the head of the list */
+		loading_deps = dep;
+	}
+
 	/* Note from feng:
 	 * new modues.dep has line like this:
 	 *	a.c32: b.32 c.c32 d.c32
@@ -401,13 +469,21 @@ int module_load_dependencies(const char *name,const char *dep_file)
 
 		if (strlen(temp_name)) {
 			char *argv[2] = { NULL, NULL };
-
-			module_load_dependencies(temp_name, MODULES_DEP);
-			if (spawn_load(temp_name, argv) < 0)
-				continue;
+			int ret;
+
+			ret = module_load_dependencies(temp_name,
+						       MODULES_DEP);
+			if (!ret) {
+				if (spawn_load(temp_name, argv) < 0)
+					continue;
+			}
 		}
 	}
 
+	/* Remove our entry from the head of loading_deps */
+	loading_deps = loading_deps->next;
+	free(dep);
+
 	return 0;
 }