[systemd-devel] [PATCH v4] systemd-analyze: filter dot output

[Lennart Poettering]

On Sat, 30.03.13 11:37, Łukasz Stelmach (stlman at poczta.fm) wrote:

> Make "systemd-analyze dot" output only lines with units matching given
> glob(7) patterns. Add --from-pattern and --to-pattern options.
> Without any patterns all relationships are printed as before.
> 
> A relationship must match the follwing expression:
> 
>     (isempty(from-pattern) || from-pattern) &&
>     (isempty(to-pattern) || to-pattern) &&
>     (isempty(P) || P[0] || P[1] || ... || P[n])
> 
> where P[N] are additional patterns provided after the "dot"
> subcommand.

Hmm, so you have a single pattern for from/to, but many that apply for
either? That's a weird assymetry?

I'd prefer if we could make all patterns a list. i.e. define a static
strv list arg_dot_patterns, another one arg_dot_from_patters, and a
final on arg_dot_to_patterns, and then apply them all.

> +                <para>Optional patterns may be given at the end. The
> +                relationship is printet if any of these matches either
> +                lefthend or righthand node.</para>

Typos: s/hend/hand/, and s/printet/printed/.

> +
>                  <para>If no command is passed <command>systemd-analyze
>                  time</command> is implied.</para>
>  
> @@ -156,6 +160,23 @@
>                                  dependencies of all these
>                                  types.</para></listitem>
>                          </varlistentry>
> +
> +                        <varlistentry>
> +                                <term><option>--from-pattern=</option></term>
> +                                <term><option>--to-pattern=</option></term>
> +
> +                                <listitem><para>When used in
> +                                conjunction with the
> +                                <command>dot</command> command (see
> +                                above), selects which relationships
> +                                are shown in the dependency graph.
> +                                They both require
> +                                <citerefentry><refentrytitle>glob</refentrytitle><manvolnum>7</manvolnum></citerefentry>
> +                                patterns as arguments, which are
> +                                matched against lefthand and
> +                                righthand, respectively, nodes of a
> +                                relationship.</para></listitem>
> +                        </varlistentry>
>                  </variablelist>
>  
>          </refsect1>
> diff --git a/src/analyze/systemd-analyze.c b/src/analyze/systemd-analyze.c
> index 01bf55e..559d588 100644
> --- a/src/analyze/systemd-analyze.c
> +++ b/src/analyze/systemd-analyze.c
> @@ -25,6 +25,7 @@
>  #include <getopt.h>
>  #include <locale.h>
>  #include <sys/utsname.h>
> +#include <fnmatch.h>
>  
>  #include "install.h"
>  #include "log.h"
> @@ -60,6 +61,8 @@ static enum dot {
>          DEP_ORDER,
>          DEP_REQUIRE
>  } arg_dot = DEP_ALL;
> +static char* arg_from_pattern=NULL;
> +static char* arg_to_pattern=NULL;

We generally put spaces before and after the =.

>  
>  struct boot_times {
>          usec_t firmware_time;
> @@ -578,7 +581,7 @@ static int analyze_time(DBusConnection *bus) {
>          return 0;
>  }
>  
> -static int graph_one_property(const char *name, const char *prop, DBusMessageIter *iter) {
> +static int graph_one_property(const char *name, const char *prop, DBusMessageIter *iter, char* patterns[]) {
>  
>          static const char * const colors[] = {
>                  "Requires",              "[color=\"black\"]",
> @@ -621,9 +624,28 @@ static int graph_one_property(const char *name, const char *prop, DBusMessageIte
>                       dbus_message_iter_get_arg_type(&sub) != DBUS_TYPE_INVALID;
>                       dbus_message_iter_next(&sub)) {
>                          const char *s;
> +                        char **p;
>  
>                          assert(dbus_message_iter_get_arg_type(&sub) == DBUS_TYPE_STRING);
>                          dbus_message_iter_get_basic(&sub, &s);
> +
> +                        if (arg_from_pattern != NULL && fnmatch(arg_from_pattern, name, 0) != 0)
> +                                continue;
> +
> +                        if (arg_to_pattern != NULL && fnmatch(arg_to_pattern, s, 0) != 0)
> +                                continue;
> +
> +                        if (*patterns == NULL) {
> +                                goto print;
> +                        }

No {} around single-line if blocks, please.

> +                        for (p=patterns; *p != NULL; p++) {

please use STRV_FOREACH for this.

> @@ -806,6 +832,14 @@ static int parse_argv(int argc, char *argv[])
>                          arg_dot = DEP_REQUIRE;
>                          break;
>  
> +                case ARG_FROM_PATTERN:
> +                        arg_from_pattern=optarg;
> +                        break;
> +
> +                case ARG_TO_PATTERN:
> +                        arg_to_pattern=optarg;
> +                        break;

Spaces around "=" please.

Otherwise looks good!

Lennart

-- 
Lennart Poettering - Red Hat, Inc.