diff mbox

kconfig: Don't leak choice names during parsing

Message ID 1519126829-7257-1-git-send-email-yamada.masahiro@socionext.com (mailing list archive)
State New, archived
Headers show

Commit Message

Masahiro Yamada Feb. 20, 2018, 11:40 a.m. UTC
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 <yamada.masahiro@socionext.com>
---

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

Comments

Ulf Magnusson Feb. 20, 2018, 7:54 p.m. UTC | #1
On Tue, Feb 20, 2018 at 12:40 PM, Masahiro Yamada
<yamada.masahiro@socionext.com> 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 <yamada.masahiro@socionext.com>
> ---
>
>  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 <ulfalizer@gmail.com>

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sam Ravnborg Feb. 20, 2018, 8:39 p.m. UTC | #2
On Tue, Feb 20, 2018 at 08:54:58PM +0100, Ulf Magnusson wrote:
> On Tue, Feb 20, 2018 at 12:40 PM, Masahiro Yamada
> <yamada.masahiro@socionext.com> 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 <yamada.masahiro@socionext.com>
> > ---
> >
> >  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 <ulfalizer@gmail.com>
> 
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Masahiro Yamada Feb. 24, 2018, 3:12 p.m. UTC | #3
2018-02-20 20:40 GMT+09:00 Masahiro Yamada <yamada.masahiro@socionext.com>:
> 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 <yamada.masahiro@socionext.com>


Applied to linux-kbuild/kconfig.
diff mbox

Patch

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());
 };