Re: [PATCH v3 2/3] kconfig: Prepare for printing reverse dependencies in groups

Ulf Magnusson
2018-02-17T16:44:09Z

On Sat, Feb 17, 2018 at 3:05 AM, Eugeniu Rosca <roscaeugeniu@gmail.com> wrote:
> From: Eugeniu Rosca <erosca@de.adit-jv.com>
>
> Currently, reverse dependencies are printed like [1]. Prepare the ground
> for printing them like [2]. No functional change is intended in this
> patch.
>
> [1] Current pattern for printing reverse dependencies:
> Selected by:
> - EXPR_A_Y /* evaluates to =y */
> - EXPR_B_N /* evaluates to =n */
> - EXPR_C_Y /* evaluates to =y */
> - EXPR_D_M /* evaluates to =m */
> - EXPR_E_N /* evaluates to =n */
>
> [2] Upcoming pattern for printing reverse dependencies:
> Selected by [y]:
> - EXPR_A_Y
> - EXPR_C_Y
> Selected by [m]:
> - EXPR_D_M
> Selected by [n]:
> - EXPR_B_N
> - EXPR_E_N
>
> Suggested-by: Ulf Magnusson <ulfalizer@gmail.com>
> Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com>
> ---
> scripts/kconfig/expr.c | 38 +++++++++++++++++++++++++++-----------
> scripts/kconfig/expr.h | 7 ++++++-
> scripts/kconfig/menu.c | 4 ++--
> 3 files changed, 35 insertions(+), 14 deletions(-)
>
> diff --git a/scripts/kconfig/expr.c b/scripts/kconfig/expr.c
> index b89baed7f15c..c610cb14284f 100644
> --- a/scripts/kconfig/expr.c
> +++ b/scripts/kconfig/expr.c
> @@ -1189,7 +1189,12 @@ expr_print_newline(struct expr *e,
> 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)
> +static void
> +__expr_print(struct expr *e,
> + void (*fn)(void *, struct symbol *, const char *),
> + void *data,
> + int prevtoken,
> + enum print_type type)
> {
> if (!e) {
> fn(data, NULL, "y");
> @@ -1201,10 +1206,16 @@ static void __expr_print(struct expr *e, void (*fn)(void *, struct symbol *, con
> switch (e->type) {
> case E_SYMBOL:
> if (e->left.sym->name)
> - if (!revdep)
> + switch (type) {
> + case PRINT_NORMAL:
> fn(data, e->left.sym, e->left.sym->name);
> - else
> + break;
> + case PRINT_REVDEP_ALL:
> expr_print_newline(e, fn, data, E_OR);
> + break;
> + default:
> + break;

Redundant 'default' case.

> + }
> else
> fn(data, NULL, "<choice>");
> break;
> @@ -1247,18 +1258,23 @@ 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:
> - __expr_print(e->left.expr, fn, data, E_OR, revdep);
> - if (!revdep)
> + __expr_print(e->left.expr, fn, data, E_OR, type);
> + if (type == PRINT_NORMAL)
> fn(data, NULL, " || ");
> - __expr_print(e->right.expr, fn, data, E_OR, revdep);
> + __expr_print(e->right.expr, fn, data, E_OR, type);
> break;
> case E_AND:
> - if (!revdep) {
> + switch (type) {
> + case PRINT_NORMAL:
> expr_print(e->left.expr, fn, data, E_AND);
> fn(data, NULL, " && ");
> expr_print(e->right.expr, fn, data, E_AND);
> - } else {
> + break;
> + case PRINT_REVDEP_ALL:
> expr_print_newline(e, fn, data, E_OR);
> + break;
> + default:
> + break;

Redundant 'default' case.

> }
> break;
> case E_LIST:
> @@ -1289,7 +1305,7 @@ static void __expr_print(struct expr *e, void (*fn)(void *, struct symbol *, con
>
> void expr_print(struct expr *e, void (*fn)(void *, struct symbol *, const char *), void *data, int prevtoken)
> {
> - __expr_print(e, fn, data, prevtoken, false);
> + __expr_print(e, fn, data, prevtoken, PRINT_NORMAL);
> }
>
> static void expr_print_file_helper(void *data, struct symbol *sym, const char *str)
> @@ -1342,7 +1358,7 @@ void expr_gstr_print(struct expr *e, struct gstr *gs)
> * line with a minus. This makes expressions much easier to read.
> * Suitable for reverse dependency expressions.
> */
> -void expr_gstr_print_revdep(struct expr *e, struct gstr *gs)
> +void expr_gstr_print_revdep(struct expr *e, struct gstr *gs, enum print_type t)
> {
> - __expr_print(e, expr_print_gstr_helper, gs, E_NONE, true);
> + __expr_print(e, expr_print_gstr_helper, gs, E_NONE, t);
> }
> diff --git a/scripts/kconfig/expr.h b/scripts/kconfig/expr.h
> index c16e82e302a2..21cb67c15091 100644
> --- a/scripts/kconfig/expr.h
> +++ b/scripts/kconfig/expr.h
> @@ -34,6 +34,11 @@ enum expr_type {
> E_LIST, E_SYMBOL, E_RANGE
> };
>
> +enum print_type {
> + PRINT_NORMAL,
> + PRINT_REVDEP_ALL,
> +};
> +
> union expr_data {
> struct expr *expr;
> struct symbol *sym;
> @@ -310,7 +315,7 @@ struct expr *expr_simplify_unmet_dep(struct expr *e1, struct expr *e2);
> void expr_fprint(struct expr *e, FILE *out);
> struct gstr; /* forward */
> void expr_gstr_print(struct expr *e, struct gstr *gs);
> -void expr_gstr_print_revdep(struct expr *e, struct gstr *gs);
> +void expr_gstr_print_revdep(struct expr *e, struct gstr *gs, enum print_type t);
>
> static inline int expr_is_yes(struct expr *e)
> {
> diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c
> index 99222855544c..5b8edba105f2 100644
> --- a/scripts/kconfig/menu.c
> +++ b/scripts/kconfig/menu.c
> @@ -828,14 +828,14 @@ static void get_symbol_str(struct gstr *r, struct symbol *sym,
> get_symbol_props_str(r, sym, P_SELECT, _(" Selects: "));
> if (sym->rev_dep.expr) {
> str_append(r, _(" Selected by: "));
> - expr_gstr_print_revdep(sym->rev_dep.expr, r);
> + expr_gstr_print_revdep(sym->rev_dep.expr, r, PRINT_REVDEP_ALL);
> str_append(r, "\n");
> }
>
> get_symbol_props_str(r, sym, P_IMPLY, _(" Implies: "));
> if (sym->implied.expr) {
> str_append(r, _(" Implied by: "));
> - expr_gstr_print_revdep(sym->implied.expr, r);
> + expr_gstr_print_revdep(sym->implied.expr, r, PRINT_REVDEP_ALL);
> str_append(r, "\n");
> }
>
> --
> 2.16.1
>

I'm not a fan of adding defaults in switches that already cover the
whole range of values (except to have an assert() in there maybe), but
it feels a bit nitpicky.

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

Cheers,
Ulf