All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] kconfig: add warn-unknown-symbols sanity check
@ 2023-08-26  7:13 Sergey Senozhatsky
  2023-08-29 13:13 ` Masahiro Yamada
  0 siblings, 1 reply; 3+ messages in thread
From: Sergey Senozhatsky @ 2023-08-26  7:13 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Nathan Chancellor, Nick Desaulniers, Nicolas Schier,
	Jonathan Corbet, Tomasz Figa, linux-kbuild, linux-doc,
	linux-kernel, Sergey Senozhatsky

Introduce KCONFIG_WARN_UNKNOWN_SYMBOLS environment variable,
which makes Kconfig warn about unknown .config symbols.

This is especially useful for continuous kernel uprevs when
some symbols can be either removed or renamed between kernel
releases (which can go unnoticed otherwise).

By default KCONFIG_WARN_UNKNOWN_SYMBOLS generates warnings,
which are non-terminal. There is an additional environment
variable KCONFIG_WERROR that overrides this behaviour and
turns warnings into errors.

Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
---
 Documentation/kbuild/kconfig.rst | 11 +++++++++++
 scripts/kconfig/confdata.c       | 23 +++++++++++++++++++++++
 2 files changed, 34 insertions(+)

diff --git a/Documentation/kbuild/kconfig.rst b/Documentation/kbuild/kconfig.rst
index 6530ecd99da3..4de1f5435b7b 100644
--- a/Documentation/kbuild/kconfig.rst
+++ b/Documentation/kbuild/kconfig.rst
@@ -56,6 +56,17 @@ KCONFIG_OVERWRITECONFIG
 If you set KCONFIG_OVERWRITECONFIG in the environment, Kconfig will not
 break symlinks when .config is a symlink to somewhere else.
 
+KCONFIG_WARN_UNKNOWN_SYMBOLS
+----------------------------
+This environment variable makes Kconfig warn about all unrecognized
+symbols in the .config file.
+
+KCONFIG_WERROR
+--------------
+If set, Kconfig will treat `KCONFIG_WARN_UNKNOWN_SYMBOLS` warnings as
+errors.
+
+
 `CONFIG_`
 ---------
 If you set `CONFIG_` in the environment, Kconfig will prefix all symbols
diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
index 992575f1e976..c24f637827fe 100644
--- a/scripts/kconfig/confdata.c
+++ b/scripts/kconfig/confdata.c
@@ -349,7 +349,12 @@ int conf_read_simple(const char *name, int def)
 	char *p, *p2;
 	struct symbol *sym;
 	int i, def_flags;
+	bool found_unknown = false;
+	const char *warn_unknown;
+	const char *werror;
 
+	warn_unknown = getenv("KCONFIG_WARN_UNKNOWN_SYMBOLS");
+	werror = getenv("KCONFIG_WERROR");
 	if (name) {
 		in = zconf_fopen(name);
 	} else {
@@ -437,6 +442,13 @@ int conf_read_simple(const char *name, int def)
 			if (def == S_DEF_USER) {
 				sym = sym_find(line + 2 + strlen(CONFIG_));
 				if (!sym) {
+					if (warn_unknown) {
+						conf_warning("unknown symbol: %s",
+							     line + 2 + strlen(CONFIG_));
+						found_unknown = true;
+						continue;
+					}
+
 					conf_set_changed(true);
 					continue;
 				}
@@ -471,6 +483,13 @@ int conf_read_simple(const char *name, int def)
 
 			sym = sym_find(line + strlen(CONFIG_));
 			if (!sym) {
+				if (warn_unknown && def != S_DEF_AUTO) {
+					conf_warning("unknown symbol: %s",
+						     line + strlen(CONFIG_));
+					found_unknown = true;
+					continue;
+				}
+
 				if (def == S_DEF_AUTO)
 					/*
 					 * Reading from include/config/auto.conf
@@ -519,6 +538,10 @@ int conf_read_simple(const char *name, int def)
 	}
 	free(line);
 	fclose(in);
+
+	if (found_unknown && werror)
+		exit(1);
+
 	return 0;
 }
 
-- 
2.42.0.rc2.253.gd59a3bf2b4-goog


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] kconfig: add warn-unknown-symbols sanity check
  2023-08-26  7:13 [PATCH] kconfig: add warn-unknown-symbols sanity check Sergey Senozhatsky
@ 2023-08-29 13:13 ` Masahiro Yamada
  2023-08-30  0:45   ` Sergey Senozhatsky
  0 siblings, 1 reply; 3+ messages in thread
From: Masahiro Yamada @ 2023-08-29 13:13 UTC (permalink / raw)
  To: Sergey Senozhatsky
  Cc: Nathan Chancellor, Nick Desaulniers, Nicolas Schier,
	Jonathan Corbet, Tomasz Figa, linux-kbuild, linux-doc,
	linux-kernel

On Sun, Aug 27, 2023 at 6:00 PM Sergey Senozhatsky
<senozhatsky@chromium.org> wrote:
>
> Introduce KCONFIG_WARN_UNKNOWN_SYMBOLS environment variable,
> which makes Kconfig warn about unknown .config symbols.
>
> This is especially useful for continuous kernel uprevs when
> some symbols can be either removed or renamed between kernel
> releases (which can go unnoticed otherwise).
>
> By default KCONFIG_WARN_UNKNOWN_SYMBOLS generates warnings,
> which are non-terminal. There is an additional environment
> variable KCONFIG_WERROR that overrides this behaviour and
> turns warnings into errors.
>
> Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
> ---
>  Documentation/kbuild/kconfig.rst | 11 +++++++++++
>  scripts/kconfig/confdata.c       | 23 +++++++++++++++++++++++
>  2 files changed, 34 insertions(+)
>
> diff --git a/Documentation/kbuild/kconfig.rst b/Documentation/kbuild/kconfig.rst
> index 6530ecd99da3..4de1f5435b7b 100644
> --- a/Documentation/kbuild/kconfig.rst
> +++ b/Documentation/kbuild/kconfig.rst
> @@ -56,6 +56,17 @@ KCONFIG_OVERWRITECONFIG
>  If you set KCONFIG_OVERWRITECONFIG in the environment, Kconfig will not
>  break symlinks when .config is a symlink to somewhere else.
>
> +KCONFIG_WARN_UNKNOWN_SYMBOLS
> +----------------------------
> +This environment variable makes Kconfig warn about all unrecognized
> +symbols in the .config file.


This warns not only for the .config but also defconfig files.

Could you reword it?

For example,

 "symbols in the config input".


> +
> +KCONFIG_WERROR
> +--------------
> +If set, Kconfig will treat `KCONFIG_WARN_UNKNOWN_SYMBOLS` warnings as
> +errors.

My hope is to turn other warnings in the config file into errors.

See below.


> +
> +
>  `CONFIG_`
>  ---------
>  If you set `CONFIG_` in the environment, Kconfig will prefix all symbols
> diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
> index 992575f1e976..c24f637827fe 100644
> --- a/scripts/kconfig/confdata.c
> +++ b/scripts/kconfig/confdata.c
> @@ -349,7 +349,12 @@ int conf_read_simple(const char *name, int def)
>         char *p, *p2;
>         struct symbol *sym;
>         int i, def_flags;
> +       bool found_unknown = false;
> +       const char *warn_unknown;
> +       const char *werror;
>
> +       warn_unknown = getenv("KCONFIG_WARN_UNKNOWN_SYMBOLS");
> +       werror = getenv("KCONFIG_WERROR");
>         if (name) {
>                 in = zconf_fopen(name);
>         } else {
> @@ -437,6 +442,13 @@ int conf_read_simple(const char *name, int def)
>                         if (def == S_DEF_USER) {
>                                 sym = sym_find(line + 2 + strlen(CONFIG_));
>                                 if (!sym) {
> +                                       if (warn_unknown) {
> +                                               conf_warning("unknown symbol: %s",
> +                                                            line + 2 + strlen(CONFIG_));
> +                                               found_unknown = true;
> +                                               continue;

Please drop this 'continue' because it would skip
conf_set_changed(true).

warn_unknown should keep the same code flow
except showing the warning message.




> +                                       }
> +
>                                         conf_set_changed(true);
>                                         continue;
>                                 }
> @@ -471,6 +483,13 @@ int conf_read_simple(const char *name, int def)
>
>                         sym = sym_find(line + strlen(CONFIG_));
>                         if (!sym) {
> +                               if (warn_unknown && def != S_DEF_AUTO) {
> +                                       conf_warning("unknown symbol: %s",
> +                                                    line + strlen(CONFIG_));
> +                                       found_unknown = true;
> +                                       continue;

Same here.
When KCONFIG_WARN_UNKNOWN_SYMBOLS is set,
conf_set_changed(true) will be skipped.



You can do like this:


@@ -471,7 +483,7 @@ int conf_read_simple(const char *name, int def)

                        sym = sym_find(line + strlen(CONFIG_));
                        if (!sym) {
-                               if (def == S_DEF_AUTO)
+                               if (def == S_DEF_AUTO) {
                                        /*
                                         * Reading from include/config/auto.conf
                                         * If CONFIG_FOO previously existed in
@@ -479,8 +491,13 @@ int conf_read_simple(const char *name, int def)
                                         * include/config/FOO must be touched.
                                         */
                                        conf_touch_dep(line + strlen(CONFIG_));
-                               else
+                               } else {
+                                       if (warn_unknown && def != S_DEF_AUTO)
+                                               conf_warning("unknown
symbol: %s",
+                                                            line +
strlen(CONFIG_));
+
                                        conf_set_changed(true);
+                               }
                                continue;
                        }





> +                               }
> +
>                                 if (def == S_DEF_AUTO)
>                                         /*
>                                          * Reading from include/config/auto.conf
> @@ -519,6 +538,10 @@ int conf_read_simple(const char *name, int def)
>         }
>         free(line);
>         fclose(in);
> +
> +       if (found_unknown && werror)
> +               exit(1);


I like to reuse 'conf_warnings' as you did in the previous version.

      if (conf_warnings && werror)
                exit(1)



Then, you do not need to add 'found_unknown'.






-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] kconfig: add warn-unknown-symbols sanity check
  2023-08-29 13:13 ` Masahiro Yamada
@ 2023-08-30  0:45   ` Sergey Senozhatsky
  0 siblings, 0 replies; 3+ messages in thread
From: Sergey Senozhatsky @ 2023-08-30  0:45 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Sergey Senozhatsky, Nathan Chancellor, Nick Desaulniers,
	Nicolas Schier, Jonathan Corbet, Tomasz Figa, linux-kbuild,
	linux-doc, linux-kernel

On (23/08/29 22:13), Masahiro Yamada wrote:
> > +KCONFIG_WARN_UNKNOWN_SYMBOLS
> > +----------------------------
> > +This environment variable makes Kconfig warn about all unrecognized
> > +symbols in the .config file.
> 
> 
> This warns not only for the .config but also defconfig files.
> 
> Could you reword it?
> 
> For example,
> 
>  "symbols in the config input".

Done.

> 
> 
> > +
> > +KCONFIG_WERROR
> > +--------------
> > +If set, Kconfig will treat `KCONFIG_WARN_UNKNOWN_SYMBOLS` warnings as
> > +errors.
> 
> My hope is to turn other warnings in the config file into errors.

Done.

> > +++ b/scripts/kconfig/confdata.c
> > @@ -349,7 +349,12 @@ int conf_read_simple(const char *name, int def)
> >         char *p, *p2;
> >         struct symbol *sym;
> >         int i, def_flags;
> > +       bool found_unknown = false;
> > +       const char *warn_unknown;
> > +       const char *werror;
> >
> > +       warn_unknown = getenv("KCONFIG_WARN_UNKNOWN_SYMBOLS");
> > +       werror = getenv("KCONFIG_WERROR");
> >         if (name) {
> >                 in = zconf_fopen(name);
> >         } else {
> > @@ -437,6 +442,13 @@ int conf_read_simple(const char *name, int def)
> >                         if (def == S_DEF_USER) {
> >                                 sym = sym_find(line + 2 + strlen(CONFIG_));
> >                                 if (!sym) {
> > +                                       if (warn_unknown) {
> > +                                               conf_warning("unknown symbol: %s",
> > +                                                            line + 2 + strlen(CONFIG_));
> > +                                               found_unknown = true;
> > +                                               continue;
> 
> Please drop this 'continue' because it would skip
> conf_set_changed(true).

My bad. Those 'continue' are left-overs from previous version.

> > +                                       }
> > +
> >                                         conf_set_changed(true);
> >                                         continue;
> >                                 }
> > @@ -471,6 +483,13 @@ int conf_read_simple(const char *name, int def)
> >
> >                         sym = sym_find(line + strlen(CONFIG_));
> >                         if (!sym) {
> > +                               if (warn_unknown && def != S_DEF_AUTO) {
> > +                                       conf_warning("unknown symbol: %s",
> > +                                                    line + strlen(CONFIG_));
> > +                                       found_unknown = true;
> > +                                       continue;
> 
> Same here.

Same here. My bad.

> > @@ -519,6 +538,10 @@ int conf_read_simple(const char *name, int def)
> >         }
> >         free(line);
> >         fclose(in);
> > +
> > +       if (found_unknown && werror)
> > +               exit(1);
> 
> 
> I like to reuse 'conf_warnings' as you did in the previous version.

Done.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2023-08-30  0:46 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-26  7:13 [PATCH] kconfig: add warn-unknown-symbols sanity check Sergey Senozhatsky
2023-08-29 13:13 ` Masahiro Yamada
2023-08-30  0:45   ` Sergey Senozhatsky

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.