[systemd-devel] PATCH: support for parsing /etc/insserv.conf for LSB system facilities

[Lennart Poettering]

On Fri, 01.07.11 11:20, Frederic Crozat (fcrozat at suse.com) wrote:

> Hi,

Heya,
> 
> as discussed on irc, I've implemented parsing /etc/insserv.conf, which,
> on SUSE (and maybe on Debian), provides LSB System Facilities (instead
> of using Provides: in LSB initscripts, for things like $network,
> $portmap, etc..).

Michael, Tollef, can you comment on this too, plz? Would be good to have
some input from you if this is useful for Debian, too.

> I didn't implement the <interactive> part of insserv.conf, since
> X-Interactive is currently broken in systemd and we'd like to keep
> sysvinit for openSUSE 12.1 (so, we could just remove X-Interactive flags
> from legacy initscripts on openSUSE, but when booting with old sysvinit,
> <interactive> list would be used to tell boot scripts which one are
> interactive).

I think the idea of X-Interactive is unfixably broken, i.e. if you end
up starting a service after boot-up at which time tty1 is blocked by a
getty, then this will necessarily deadlock. I am not sure how this could
ever be fixed.

> +                t = strstrip(l);
> +                if ((*t != '$') && (*t != '<'))

Please don't place redundant brackets here. 

> +                        continue;
> +
> +                parsed = strv_split (t,WHITESPACE);

Please no spaces between a function and the opening bracket. This is
good: sin(x), this is bad: sin (x).

> +                /* we ignore <interactive>, not used, equivalent to X-Interactive */
> +                if (parsed && !startswith_no_case (parsed[0], "<interactive>")) {
> +                        char *facility;
> +                        Unit *u;
> +                        if (sysv_translate_facility(parsed[0], NULL, &facility) < 0)
> +                                continue;
> +                        if ((u = manager_get_unit (mgr, facility)) && (u->meta.type == UNIT_TARGET)) {
> +                                UnitDependency e;
> +                                char *dep = NULL, *name;
> +                                int j;
> +
> +                                for (j=1;parsed[j]; j++) {

Please use STRV_FOREACH for iterating through string arrays.

Looks good otherwise. I'd prefer Michael's and Tollef's comments on the
benefits of this though before I merge this.

Lennart

-- 
Lennart Poettering - Red Hat, Inc.