On Sat, Aug 15, 2009 at 05:19:41PM +0200, Petter Reinholdtsen wrote:
> [Raphael Geissert]
> > As mentioned on IRC, when a virtual facility depends on itself
> > insserv enters an infinite loop and starts leaking memory.
> 
> Thank you.  I've written this test case to reproduce the problem.  CC
> to upstream to let him know about the problem.

OK, I've added a check to avoid such loops and also check to
avoid that a scripts depends on a scripts which uses the
system facility `$all' in its Required-Start.

Does this work for you?


     Werner

-- 
  "Having a smoking section in a restaurant is like having
          a peeing section in a swimming pool." -- Edward Burr
--- insserv.c
+++ insserv.c	2009-08-17 14:00:25.841900763 +0200
@@ -222,6 +222,7 @@ typedef struct string {
 
 typedef struct repl {
     list_t     r_list;
+    ushort	flags;
     string_t	 r[1];
 } __align repl_t;
 #define getrepl(arg)	list_entry((arg), struct repl, r_list)
@@ -2060,13 +2061,19 @@ static void expand_faci(list_t *restrict
 
 	list_for_each_safe(tmp, safe, ptr) {
 	    repl_t * rnxt = getrepl(tmp);
+	    if (rnxt->flags & 0x0001) {
+		error("Loop detected during expanding system facilities in the insserv.conf file(s): %s\n",
+		      rnxt->r[0].name);
+	    }
 	    if (*rnxt->r[0].name == '$') {
 		if (*deep > 10) {
 		    warn("The nested level of the system facilities in the insserv.conf file(s) is to large\n");
 		    goto out;
 		}
 		(*deep)++;
+		rnxt->flags |= 0x0001;
 		expand_faci(tmp, head, deep);
+		rnxt->flags &= ~0x0001;
 		(*deep)--;
 	    } else if (*deep > 0) {
 		repl_t *restrict subst;
@@ -2087,9 +2094,12 @@ static inline void expand_conf(void)
     list_for_each(ptr, sysfaci_start) {
 	list_t * rlist, * safe, * head = &getfaci(ptr)->replace;
 	list_for_each_safe(rlist, safe, head) {
-	    if (*getrepl(rlist)->r[0].name == '$') {
+	    repl_t * tmp = getrepl(rlist);
+	    if (*tmp->r[0].name == '$') {
 		int deep = 0;
+		tmp->flags |= 0x0001;
 		expand_faci(rlist, rlist, &deep);
+		tmp->flags &= ~0x0001;
 	    }
 	}
     }
--- listing.c
+++ listing.c	2009-08-17 14:15:48.893900934 +0200
@@ -364,6 +364,7 @@ static void __follow (dir_t *restrict di
 	 * Do not count the dependcy deep of the system facilities
 	 * but follow them to count the replacing provides.
 	 */
+
 	if (*ptmp->name == '$')
 	    warn("System facilities not fully expanded, see %s!\n", dir->name);
 	else if (++deep > MAX_DEEP) {
@@ -398,6 +399,12 @@ static void __follow (dir_t *restrict di
 		break;				/* Loop detected, stop recursion */
 	    }
 
+	    if ((mode == 'S') && (attof(tmp)->flags & SERV_ALL)) {
+		warn("%s depends on %s and therefore on system facility `$all' which can not be true!\n",
+		     target->script ? target->script : target->name, tmp->script ? tmp->script : tmp->name);
+		continue;
+	    }
+
 	    if (ptrg->deep >= deep)		/* Nothing new */
 		continue;
 						/* The inner recursion */

Reply via email to