On Mon, 15.10.12 00:33, Michael Stapelberg (stapelb...@debian.org) wrote:

Heya,

I am not a big fan of heuristics like this, but it does fix a valid
issue and I don't see how else we could implement this. So, yeah, looks
mostly good to commit. But first, please make a few changes:

> +                        /* Try to figure out whether this init script 
> supports
> +                         * the reload operation. This heuristic looks for
> +                         * "Usage: " lines which include the reload option. 
> */
> +                        if ( state == USAGE_CONTINUATION ||
> +                            (state == NORMAL && strcasestr(t, "usage"))) {
> +                                if (strcasestr(t, "{reload|") ||
> +                                    strcasestr(t, "{reload}") ||
> +                                    strcasestr(t, "{reload\"") ||
> +                                    strcasestr(t, "|reload|") ||
> +                                    strcasestr(t, "|reload}") ||
> +                                    strcasestr(t, "|reload\"")) {

I'd prefer if this complex check (at least the last part of it) could be
moved into its own function, to make this easier to read.

> +                                        supports_reload = true;
> +                                        state = NORMAL;
> +                                } else if (t[strlen(t)-1] == '\\') {
> +                                    state = USAGE_CONTINUATION;
> +                                } else {
> +                                    state = NORMAL;
> +                                }

We tend to write:

if (...) 
        foo;
else
        bar;

rather than:

if (...) {
        foo;
} else {
        bar;
}

C is not PHP after all ;-)

Lennart

-- 
Lennart Poettering - Red Hat, Inc.


-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Reply via email to