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 */