2018-02-20 11:42:44

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH] kconfig: Don't leak choice names during parsing

The named choice is not used in the kernel tree, but if it were used,
it would not be freed.

The intention of the named choice can be seen in the log of
commit 5a1aa8a1aff6 ("kconfig: add named choice group").

There is room for argument if this is useful in practice, but anyway
I am fixing the memory leak.

Signed-off-by: Masahiro Yamada <[email protected]>
---

scripts/kconfig/zconf.y | 1 +
1 file changed, 1 insertion(+)

diff --git a/scripts/kconfig/zconf.y b/scripts/kconfig/zconf.y
index 4be9805..4893676 100644
--- a/scripts/kconfig/zconf.y
+++ b/scripts/kconfig/zconf.y
@@ -276,6 +276,7 @@ choice: T_CHOICE word_opt T_EOL
sym->flags |= SYMBOL_AUTO;
menu_add_entry(sym);
menu_add_expr(P_CHOICE, NULL, NULL);
+ free($2);
printd(DEBUG_PARSE, "%s:%d:choice\n", zconf_curname(), zconf_lineno());
};

--
2.7.4



2018-02-20 19:55:49

by Ulf Magnusson

[permalink] [raw]
Subject: Re: [PATCH] kconfig: Don't leak choice names during parsing

On Tue, Feb 20, 2018 at 12:40 PM, Masahiro Yamada
<[email protected]> wrote:
> The named choice is not used in the kernel tree, but if it were used,
> it would not be freed.
>
> The intention of the named choice can be seen in the log of
> commit 5a1aa8a1aff6 ("kconfig: add named choice group").
>
> There is room for argument if this is useful in practice, but anyway
> I am fixing the memory leak.
>
> Signed-off-by: Masahiro Yamada <[email protected]>
> ---
>
> scripts/kconfig/zconf.y | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/scripts/kconfig/zconf.y b/scripts/kconfig/zconf.y
> index 4be9805..4893676 100644
> --- a/scripts/kconfig/zconf.y
> +++ b/scripts/kconfig/zconf.y
> @@ -276,6 +276,7 @@ choice: T_CHOICE word_opt T_EOL
> sym->flags |= SYMBOL_AUTO;
> menu_add_entry(sym);
> menu_add_expr(P_CHOICE, NULL, NULL);
> + free($2);
> printd(DEBUG_PARSE, "%s:%d:choice\n", zconf_curname(), zconf_lineno());
> };
>
> --
> 2.7.4
>

Reviewed-by: Ulf Magnusson <[email protected]>

Only place I've seen named choices in practice is in the esp-idf
project. Not sure what they're using them for.

They were handy for writing tests too. :P

Cheers,
Ulf

2018-02-20 20:40:01

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [PATCH] kconfig: Don't leak choice names during parsing

On Tue, Feb 20, 2018 at 08:54:58PM +0100, Ulf Magnusson wrote:
> On Tue, Feb 20, 2018 at 12:40 PM, Masahiro Yamada
> <[email protected]> wrote:
> > The named choice is not used in the kernel tree, but if it were used,
> > it would not be freed.
> >
> > The intention of the named choice can be seen in the log of
> > commit 5a1aa8a1aff6 ("kconfig: add named choice group").
> >
> > There is room for argument if this is useful in practice, but anyway
> > I am fixing the memory leak.
> >
> > Signed-off-by: Masahiro Yamada <[email protected]>
> > ---
> >
> > scripts/kconfig/zconf.y | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/scripts/kconfig/zconf.y b/scripts/kconfig/zconf.y
> > index 4be9805..4893676 100644
> > --- a/scripts/kconfig/zconf.y
> > +++ b/scripts/kconfig/zconf.y
> > @@ -276,6 +276,7 @@ choice: T_CHOICE word_opt T_EOL
> > sym->flags |= SYMBOL_AUTO;
> > menu_add_entry(sym);
> > menu_add_expr(P_CHOICE, NULL, NULL);
> > + free($2);
> > printd(DEBUG_PARSE, "%s:%d:choice\n", zconf_curname(), zconf_lineno());
> > };
> >
> > --
> > 2.7.4
> >
>
> Reviewed-by: Ulf Magnusson <[email protected]>
>
> Only place I've seen named choices in practice is in the esp-idf
> project. Not sure what they're using them for.

The plan with named choices was that it should
allow one to do something like this:

# in one file the choice was defined:
choice FOOBAR
prompt "foobar"

config SAMBAR
bool "bar"

endchoice


# and in another file the choice was extended:
choice FOOBAR

config SAMFOO
bool "foo"

endchoice

The user was then presented only with a single list of choices.

The idea was that this would allow better support for a base
configuration that could be extended in another file.
Think arch/Kconfig versus an arch specific Kconfig file.

But Roman nor I never got around to actually implement this.
Most likely because the need was not big enough.

Sam

2018-02-24 15:14:06

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH] kconfig: Don't leak choice names during parsing

2018-02-20 20:40 GMT+09:00 Masahiro Yamada <[email protected]>:
> The named choice is not used in the kernel tree, but if it were used,
> it would not be freed.
>
> The intention of the named choice can be seen in the log of
> commit 5a1aa8a1aff6 ("kconfig: add named choice group").
>
> There is room for argument if this is useful in practice, but anyway
> I am fixing the memory leak.
>
> Signed-off-by: Masahiro Yamada <[email protected]>


Applied to linux-kbuild/kconfig.




--
Best Regards
Masahiro Yamada