Hi Paul,
I tested your patch and it works properly for me. It even still applies
on linux-next. Would it be possible to extend your patch to also check
symbols in other statements (depends on, if, etc.)?
Best regards,
Valentin
On Nov 17 '16 09:58, Paul Bolle wrote:
> [Added Michal]
>
> Hi Valentin,
>
> On Wed, 2016-11-16 at 08:31 +0100, Valentin Rothberg wrote:
> > your commit c3ca951fe41a ("i2c: Add Tegra BPMP I2C proxy driver")
> > popped up in today's linux-next tree, adding Kconfig option
> > I2C_TEGRA_BPMP, which further selects I2C_ALGO_BUSCLEAR, which is
> > nowhere defined in Kconfig.
> >
> > Is there a patch queued somewhere to add I2C_ALGO_BUSCLEAR to Kconfig?
> > I could not find anything on the lkml; only some older repositories
> > on github, where the options is defined in drivers/i2c/busses/Kconfig.
>
> Attached is a v2 of a patch I submitted way to long ago. It adds a warning for
> exactly this issue. That should help others to spot them. (It's a bit smarter
> than your check kconfig script as it also warns if you select symbols that are
> outside of you arch's scope. Ie, not only for entirely unknown symbols.)
>
> Please play with it. Unless you spot some issues I really should be properly
> resubmitting it.
>
> Thanks,
>
>
> Paul Bolle
>
> -------- >8 >8 >8 >8 -------
> From: Paul Bolle <pebo...@tiscali.nl>
> Subject: [PATCH] kconfig: warn if an unknown symbol is selected
>
> A select of an unknown symbol is basically treated as a nop and is
> silently skipped. This is annoying if the selected symbol contains a
> typo. It can also hide the fact that a treewide symbol cleanup was only
> done partially.
>
> There are also a few cases were this might have been done on purpose.
> But that anti-pattern should be discouraged. Almost all select
> statements point to a known and reachable symbol. So people will likely
> assume that any selected symbol is actually set. Selects that violate
> this assumption can only be spotted by checking multiple Kconfig files,
> often across architectures. It is unlikely that people will do this
> regularily.
>
> So let's warn when we notice a select of a symbol that is not known in
> the configuration we're creating.
>
> Signed-off-by: Paul Bolle <pebo...@tiscali.nl>
> Acked-by: Michal Marek <mma...@suse.cz> (v1)
> ---
> scripts/kconfig/conf.c | 42 ++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 42 insertions(+)
>
> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
> index 866369f10ff8..55a4dacb196d 100644
> --- a/scripts/kconfig/conf.c
> +++ b/scripts/kconfig/conf.c
> @@ -447,6 +447,46 @@ static void check_conf(struct menu *menu)
> check_conf(child);
> }
>
> +static void check_selects(struct menu *menu)
> +{
> + struct symbol *sym, *sel;
> + struct property *prop;
> +
> + while (menu) {
> + sym = menu->sym;
> +
> + if (sym && !sym_is_choice(sym) &&
> + sym->type != S_UNKNOWN &&
> + sym_get_tristate_value(sym) != no &&
> + sym->flags & SYMBOL_WRITE) {
> + for_all_properties(sym, prop, P_SELECT) {
> + sel = prop->expr->left.sym;
> + if (sel->type == S_UNKNOWN &&
> + expr_calc_value(prop->visible.expr) != no) {
> + fprintf(stderr, "%s:%d:warning: ",
> + prop->file->name,
> + prop->lineno);
> + fprintf(stderr,
> + "'%s' selects unknown symbol
> '%s'\n",
> + sym->name,
> + sel->name);
> + }
> + }
> + }
> +
> + if (menu->list) {
> + menu = menu->list;
> + } else if (menu->next) {
> + menu = menu->next;
> + } else while ((menu = menu->parent)) {
> + if (menu->next) {
> + menu = menu->next;
> + break;
> + }
> + }
> + }
> +}
> +
> static struct option long_opts[] = {
> {"oldaskconfig", no_argument, NULL, oldaskconfig},
> {"oldconfig", no_argument, NULL, oldconfig},
> @@ -686,6 +726,8 @@ int main(int ac, char **av)
> break;
> }
>
> + check_selects(rootmenu.list);
> +
> if (sync_kconfig) {
> /* silentoldconfig is used during the build so we shall update
> autoconf.
> * All other commands are only used to generate a config.
> --
> 2.7.4
