Re: [PATCH 1/2] kconfig: Print reverse dependencies on new line consistently

Ulf Magnusson
2018-02-13T06:11:32Z

On Tue, Feb 13, 2018 at 1:56 AM, Eugeniu Rosca <roscaeugeniu@gmail.com> wrote:
> From: Eugeniu Rosca <erosca@de.adit-jv.com>
>
> Commit 1ccb27143360 ("kconfig: make "Selected by:" and "Implied by:"
> readable") made an incredible improvement in how reverse dependencies
> are perceived by the user, by breaking down the single (often
> interminable) expression string into small readable chunks, each of
> them displayed on a separate line:
>
> Selected by:
> - A & B
> - C & (D || E)
>
> Unfortunately, what happens with the non-OR (either E_SYMBOL or E_AND)
> expressions is that they don't get a dedicated line:
>
> Selected by: F & G
>
> That's arguably a bug/misbehavior, but it makes the implementation of
> something like below more complicated:
>
> Selected by:
> - [m] F & G /* where (F & G) evaluates to '=m' */
>
> Adding '[m]', '[y]' or '[ ]' to the left side of each reverse dependency
> is subject of a different commit. The purpose of current commit is to
> print the 'Selected by:' and 'Implied by:' expressions on a separate
> line _consistently_.
>
> An example of change contributed by this commit is seen below.
>
> │ Symbol: NEED_SG_DMA_LENGTH [=y]
> │ ...
> │ Selected by: IOMMU_DMA [=y] && IOMMU_SUPPORT [=y]
>
> becomes:
>
> │ Symbol: NEED_SG_DMA_LENGTH [=y]
> │ ...
> │ Selected by:
> │ - IOMMU_DMA [=y] && IOMMU_SUPPORT [=y]
>
> This patch has been tested using a custom variant of zconfdump which
> prints the reverse dependencies for each config symbol.
>
> Suggested-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> Suggested-by: Ulf Magnusson <ulfalizer@gmail.com>
> Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com>
> ---
> scripts/kconfig/expr.c | 31 ++++++++++++++++++++++---------
> 1 file changed, 22 insertions(+), 9 deletions(-)
>
> diff --git a/scripts/kconfig/expr.c b/scripts/kconfig/expr.c
> index d45381986ac7..0704bcdf4c78 100644
> --- a/scripts/kconfig/expr.c
> +++ b/scripts/kconfig/expr.c
> @@ -1179,6 +1179,16 @@ struct expr *expr_simplify_unmet_dep(struct expr *e1, struct expr *e2)
> return expr_get_leftmost_symbol(ret);
> }
>
> +static void
> +expr_print_newline(struct expr *e,
> + void (*fn)(void *, struct symbol *, const char *),
> + void *data,
> + int prevtoken)
> +{
> + fn(data, NULL, "\n - ");
> + expr_print(e, fn, data, prevtoken);
> +}
> +
> static void __expr_print(struct expr *e, void (*fn)(void *, struct symbol *, const char *), void *data, int prevtoken, bool revdep)
> {
> if (!e) {
> @@ -1191,7 +1201,10 @@ static void __expr_print(struct expr *e, void (*fn)(void *, struct symbol *, con
> switch (e->type) {
> case E_SYMBOL:
> if (e->left.sym->name)
> - fn(data, e->left.sym, e->left.sym->name);
> + if (revdep)
> + expr_print_newline(e, fn, data, E_OR);
> + else
> + fn(data, e->left.sym, e->left.sym->name);
> else
> fn(data, NULL, "<choice>");
> break;
> @@ -1234,19 +1247,19 @@ static void __expr_print(struct expr *e, void (*fn)(void *, struct symbol *, con
> fn(data, e->right.sym, e->right.sym->name);
> break;
> case E_OR:
> - if (revdep && e->left.expr->type != E_OR)
> - fn(data, NULL, "\n - ");
> __expr_print(e->left.expr, fn, data, E_OR, revdep);
> - if (revdep)
> - fn(data, NULL, "\n - ");
> - else
> + if (!revdep)
> fn(data, NULL, " || ");
> __expr_print(e->right.expr, fn, data, E_OR, revdep);
> break;
> case E_AND:
> - expr_print(e->left.expr, fn, data, E_AND);
> - fn(data, NULL, " && ");
> - expr_print(e->right.expr, fn, data, E_AND);
> + if (revdep) {
> + expr_print_newline(e, fn, data, E_OR);
> + } else {
> + expr_print(e->left.expr, fn, data, E_AND);
> + fn(data, NULL, " && ");
> + expr_print(e->right.expr, fn, data, E_AND);
> + }
> break;
> case E_LIST:
> fn(data, e->right.sym, e->right.sym->name);
> --
> 2.16.1
>

Reviewed-by: Ulf Magnusson <ulfalizer@gmail.com>

Cheers,
Ulf