On Sun, Feb 18, 2018 at 12:07:02PM +0100, Eugeniu Rosca wrote:
> If expr_print_newline is renamed to expr_print_{select,revdep}, then
> I still face the need of expr_print_newline. So, I kept the *newline()
> function in place and came up with below solution to aggregate
> duplicated code. Please, let me know if it looks OK to you (btw, it
> creates a slightly higher --stat compared to current solution).
>
> diff --git a/scripts/kconfig/expr.c b/scripts/kconfig/expr.c
> index 48b99371d276..a6316e5681d9 100644
> --- a/scripts/kconfig/expr.c
> +++ b/scripts/kconfig/expr.c
> @@ -1189,6 +1189,34 @@ expr_print_newline(struct expr *e,
> expr_print(e, fn, data, prevtoken);
> }
>
> +static void
> +expr_print_revdep(struct expr *e,
> + void (*fn)(void *, struct symbol *, const char *),
> + void *data,
> + int prevtoken,
> + enum print_type type)
> +{
> + switch (type) {
> + case PRINT_REVDEP_ALL:
> + expr_print_newline(e, fn, data, prevtoken);
> + break;
> + case PRINT_REVDEP_YES:
> + if (expr_calc_value(e) == yes)
> + expr_print_newline(e, fn, data, prevtoken);
> + break;
> + case PRINT_REVDEP_MOD:
> + if (expr_calc_value(e) == mod)
> + expr_print_newline(e, fn, data, prevtoken);
> + break;
> + case PRINT_REVDEP_NO:
> + if (expr_calc_value(e) == no)
> + expr_print_newline(e, fn, data, prevtoken);
> + break;
> + default:
> + break;
> + }
> +}
> +
I think it's fine to have a separate expr_print_newline() function
still. Getting rid of the repetition still makes it more readable to me.
Maybe you could do something like this if you want to eliminate
expr_print_newline() though. AFAICS, it isn't used outside
expr_print_revdep().
static void
expr_print_revdep(struct expr *e,
void (*fn)(void *, struct symbol *, const char *),
void *data,
int prevtoken,
enum print_type type)
{
if (type == PRINT_REVDEP_ALL ||
type == PRINT_REVDEP_YES && expr_calc_value(e) == yes ||
type == PRINT_REVDEP_MOD && expr_calc_value(e) == mod ||
type == PRINT_REVDEP_NO && expr_calc_value(e) == no) {
fn(data, NULL, "\n - ");
expr_print(e, fn, data, prevtoken);
}
}
Could turn that into a switch over 'type' and set a 'print_revdep' flag
which is checked at the end too, if you'd prefer that:
switch (type) {
...
case PRINT_REVDEP_YES:
print_revdep = (expr_calc_value(e) == yes);
break;
...
}
if (print_revdep) {
fn(data, NULL, "\n - ");
expr_print(e, fn, data, prevtoken);
}
Or return early:
case PRINT_REVDEP_YES:
if (expr_calc_value(e) != yes)
return;
break;
...
}
fn(data, NULL, "\n - ");
expr_print(e, fn, data, prevtoken);
The original version is already fine by me though. The 'if' version
seems pretty direct too.
Cheers,
Ulf
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
