linux-kbuild.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5] kconfig: avoid an infinite loop in oldconfig/syncconfig
@ 2023-11-04 22:27 Yoann Congal
  2023-11-05  7:55 ` Masahiro Yamada
  0 siblings, 1 reply; 5+ messages in thread
From: Yoann Congal @ 2023-11-04 22:27 UTC (permalink / raw)
  To: Masahiro Yamada, linux-kbuild
  Cc: linux-kernel, Randy Dunlap, Yoann Congal, Brandon Maier

Exit on error when asking for value and reading stdin returns an error
(mainly if it has reached EOF or is closed).

This infinite loop happens in particular for hex/int configs without an
explicit default value.

Previously, this case would loop:
* oldconfig prompts for the value but stdin has reached EOF
* It gets the global default value : an empty string
* This is not a valid hex/int value so it prompts again, hence the
  infinite loop.

This case happens with a configuration like this (a hex config without a
valid default value):
  config TEST_KCONFIG
       hex "Test KConfig"
       # default 0x0

And using:
  make oldconfig < /dev/null

This was discovered when working on Yocto bug[0] on a downstream
kconfig user (U-boot)

[0]: https://bugzilla.yoctoproject.org/show_bug.cgi?id=14136

CC: Brandon Maier <brandon.maier@collins.com>
Signed-off-by: Yoann Congal <yoann.congal@smile.fr>
---
v4->v5:
 * Switched to Masahiro Yamada's suggested code.
v3->v4:
 * Added Brandon Maier's "Tested-by". Thanks!
v2->v3:
 * Simplify the patch by fusing comments of :
   * Masahiro Yamada : Exit as soon as reading stdin hits an error
   * Randy Dunlap : Display the name of the currently read symbol
v1->v2:
 * Improve coding style
 * Put more info in the commit message
---
 scripts/kconfig/conf.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
index 33d19e419908..62de1fbaff97 100644
--- a/scripts/kconfig/conf.c
+++ b/scripts/kconfig/conf.c
@@ -76,8 +76,10 @@ static void strip(char *str)
 /* Helper function to facilitate fgets() by Jean Sacren. */
 static void xfgets(char *str, int size, FILE *in)
 {
-	if (!fgets(str, size, in))
+	if (!fgets(str, size, in)) {
 		fprintf(stderr, "\nError in reading or end of file.\n");
+		exit(1);
+	}
 
 	if (!tty_stdio)
 		printf("%s", str);
-- 
2.30.2


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

* Re: [PATCH v5] kconfig: avoid an infinite loop in oldconfig/syncconfig
  2023-11-04 22:27 [PATCH v5] kconfig: avoid an infinite loop in oldconfig/syncconfig Yoann Congal
@ 2023-11-05  7:55 ` Masahiro Yamada
  2023-11-07 21:00   ` Nathan Chancellor
  0 siblings, 1 reply; 5+ messages in thread
From: Masahiro Yamada @ 2023-11-05  7:55 UTC (permalink / raw)
  To: Yoann Congal; +Cc: linux-kbuild, linux-kernel, Randy Dunlap, Brandon Maier

On Sun, Nov 5, 2023 at 7:27 AM Yoann Congal <yoann.congal@smile.fr> wrote:
>
> Exit on error when asking for value and reading stdin returns an error
> (mainly if it has reached EOF or is closed).
>
> This infinite loop happens in particular for hex/int configs without an
> explicit default value.
>
> Previously, this case would loop:
> * oldconfig prompts for the value but stdin has reached EOF
> * It gets the global default value : an empty string
> * This is not a valid hex/int value so it prompts again, hence the
>   infinite loop.
>
> This case happens with a configuration like this (a hex config without a
> valid default value):
>   config TEST_KCONFIG
>        hex "Test KConfig"
>        # default 0x0
>
> And using:
>   make oldconfig < /dev/null
>
> This was discovered when working on Yocto bug[0] on a downstream
> kconfig user (U-boot)
>
> [0]: https://bugzilla.yoctoproject.org/show_bug.cgi?id=14136
>
> CC: Brandon Maier <brandon.maier@collins.com>
> Signed-off-by: Yoann Congal <yoann.congal@smile.fr>

Applied to linux-kbuild.
Thanks.


> ---
> v4->v5:
>  * Switched to Masahiro Yamada's suggested code.
> v3->v4:
>  * Added Brandon Maier's "Tested-by". Thanks!
> v2->v3:
>  * Simplify the patch by fusing comments of :
>    * Masahiro Yamada : Exit as soon as reading stdin hits an error
>    * Randy Dunlap : Display the name of the currently read symbol
> v1->v2:
>  * Improve coding style
>  * Put more info in the commit message
> ---
>  scripts/kconfig/conf.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
> index 33d19e419908..62de1fbaff97 100644
> --- a/scripts/kconfig/conf.c
> +++ b/scripts/kconfig/conf.c
> @@ -76,8 +76,10 @@ static void strip(char *str)
>  /* Helper function to facilitate fgets() by Jean Sacren. */
>  static void xfgets(char *str, int size, FILE *in)
>  {
> -       if (!fgets(str, size, in))
> +       if (!fgets(str, size, in)) {
>                 fprintf(stderr, "\nError in reading or end of file.\n");
> +               exit(1);
> +       }
>
>         if (!tty_stdio)
>                 printf("%s", str);
> --
> 2.30.2
>


-- 
Best Regards
Masahiro Yamada

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

* Re: [PATCH v5] kconfig: avoid an infinite loop in oldconfig/syncconfig
  2023-11-05  7:55 ` Masahiro Yamada
@ 2023-11-07 21:00   ` Nathan Chancellor
  2023-11-09  4:26     ` Masahiro Yamada
  0 siblings, 1 reply; 5+ messages in thread
From: Nathan Chancellor @ 2023-11-07 21:00 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Yoann Congal, linux-kbuild, linux-kernel, Randy Dunlap,
	Brandon Maier, Nick Desaulniers, llvm

On Sun, Nov 05, 2023 at 04:55:57PM +0900, Masahiro Yamada wrote:
> On Sun, Nov 5, 2023 at 7:27 AM Yoann Congal <yoann.congal@smile.fr> wrote:
> >
> > Exit on error when asking for value and reading stdin returns an error
> > (mainly if it has reached EOF or is closed).
> >
> > This infinite loop happens in particular for hex/int configs without an
> > explicit default value.
> >
> > Previously, this case would loop:
> > * oldconfig prompts for the value but stdin has reached EOF
> > * It gets the global default value : an empty string
> > * This is not a valid hex/int value so it prompts again, hence the
> >   infinite loop.
> >
> > This case happens with a configuration like this (a hex config without a
> > valid default value):
> >   config TEST_KCONFIG
> >        hex "Test KConfig"
> >        # default 0x0
> >
> > And using:
> >   make oldconfig < /dev/null
> >
> > This was discovered when working on Yocto bug[0] on a downstream
> > kconfig user (U-boot)
> >
> > [0]: https://bugzilla.yoctoproject.org/show_bug.cgi?id=14136
> >
> > CC: Brandon Maier <brandon.maier@collins.com>
> > Signed-off-by: Yoann Congal <yoann.congal@smile.fr>
> 
> Applied to linux-kbuild.
> Thanks.

For what it's worth, this change breaks our continuous integration [1]
because tuxmake explicitly uses /dev/null as stdin to make for
non-interactive commands (I think it does this as basically the
equivalent of "yes '' | make" in Python), so the error will always
occur.

Before:

$ curl -LSso .config https://gitlab.archlinux.org/archlinux/packaging/packages/linux/-/raw/main/config

# Same as 'make < /dev/null' but still
$ python3 -c "import subprocess; subprocess.run(['make', '-j128'], stdin=subprocess.DEVNULL)"
  SYNC    include/config/auto.conf
  HOSTCC  scripts/basic/fixdep
  HOSTCC  scripts/kconfig/conf.o
  HOSTCC  scripts/kconfig/confdata.o
  HOSTCC  scripts/kconfig/expr.o
  LEX     scripts/kconfig/lexer.lex.c
  YACC    scripts/kconfig/parser.tab.[ch]
  HOSTCC  scripts/kconfig/menu.o
  HOSTCC  scripts/kconfig/preprocess.o
  HOSTCC  scripts/kconfig/symbol.o
  HOSTCC  scripts/kconfig/util.o
  HOSTCC  scripts/kconfig/lexer.lex.o
  HOSTCC  scripts/kconfig/parser.tab.o
  HOSTLD  scripts/kconfig/conf
*
* Restart config...
*
...
Error in reading or end of file.

  GEN     arch/x86/include/generated/asm/orc_hash.h
  WRAP    arch/x86/include/generated/uapi/asm/bpf_perf_event.h
  WRAP    arch/x86/include/generated/uapi/asm/errno.h
...

After:

$ curl -LSso .config https://gitlab.archlinux.org/archlinux/packaging/packages/linux/-/raw/main/config

$ python3 -c "import subprocess; subprocess.run(['make', '-j128'], stdin=subprocess.DEVNULL)"
  SYNC    include/config/auto.conf
  HOSTCC  scripts/basic/fixdep
  HOSTCC  scripts/kconfig/conf.o
  HOSTCC  scripts/kconfig/confdata.o
  HOSTCC  scripts/kconfig/expr.o
  LEX     scripts/kconfig/lexer.lex.c
  YACC    scripts/kconfig/parser.tab.[ch]
  HOSTCC  scripts/kconfig/menu.o
  HOSTCC  scripts/kconfig/preprocess.o
  HOSTCC  scripts/kconfig/symbol.o
  HOSTCC  scripts/kconfig/util.o
  HOSTCC  scripts/kconfig/lexer.lex.o
  HOSTCC  scripts/kconfig/parser.tab.o
  HOSTLD  scripts/kconfig/conf
*
* Restart config...
*
...
Error in reading or end of file.
make[3]: *** [scripts/kconfig/Makefile:77: syncconfig] Error 1
...

We have been doing this for some time and never run across an infinite
loop in syncconfig. Can this be improved?

[1]: https://github.com/ClangBuiltLinux/continuous-integration2/actions/runs/6784349581

Cheers,
Nathan

> > ---
> > v4->v5:
> >  * Switched to Masahiro Yamada's suggested code.
> > v3->v4:
> >  * Added Brandon Maier's "Tested-by". Thanks!
> > v2->v3:
> >  * Simplify the patch by fusing comments of :
> >    * Masahiro Yamada : Exit as soon as reading stdin hits an error
> >    * Randy Dunlap : Display the name of the currently read symbol
> > v1->v2:
> >  * Improve coding style
> >  * Put more info in the commit message
> > ---
> >  scripts/kconfig/conf.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
> > index 33d19e419908..62de1fbaff97 100644
> > --- a/scripts/kconfig/conf.c
> > +++ b/scripts/kconfig/conf.c
> > @@ -76,8 +76,10 @@ static void strip(char *str)
> >  /* Helper function to facilitate fgets() by Jean Sacren. */
> >  static void xfgets(char *str, int size, FILE *in)
> >  {
> > -       if (!fgets(str, size, in))
> > +       if (!fgets(str, size, in)) {
> >                 fprintf(stderr, "\nError in reading or end of file.\n");
> > +               exit(1);
> > +       }
> >
> >         if (!tty_stdio)
> >                 printf("%s", str);
> > --
> > 2.30.2
> >
> 
> 
> -- 
> Best Regards
> Masahiro Yamada

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

* Re: [PATCH v5] kconfig: avoid an infinite loop in oldconfig/syncconfig
  2023-11-07 21:00   ` Nathan Chancellor
@ 2023-11-09  4:26     ` Masahiro Yamada
  2023-11-09 22:56       ` Yoann Congal
  0 siblings, 1 reply; 5+ messages in thread
From: Masahiro Yamada @ 2023-11-09  4:26 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Yoann Congal, linux-kbuild, linux-kernel, Randy Dunlap,
	Brandon Maier, Nick Desaulniers, llvm

On Tue, Nov 7, 2023 at 11:00 PM Nathan Chancellor <nathan@kernel.org> wrote:
>
> On Sun, Nov 05, 2023 at 04:55:57PM +0900, Masahiro Yamada wrote:
> > On Sun, Nov 5, 2023 at 7:27 AM Yoann Congal <yoann.congal@smile.fr> wrote:
> > >
> > > Exit on error when asking for value and reading stdin returns an error
> > > (mainly if it has reached EOF or is closed).
> > >
> > > This infinite loop happens in particular for hex/int configs without an
> > > explicit default value.
> > >
> > > Previously, this case would loop:
> > > * oldconfig prompts for the value but stdin has reached EOF
> > > * It gets the global default value : an empty string
> > > * This is not a valid hex/int value so it prompts again, hence the
> > >   infinite loop.
> > >
> > > This case happens with a configuration like this (a hex config without a
> > > valid default value):
> > >   config TEST_KCONFIG
> > >        hex "Test KConfig"
> > >        # default 0x0
> > >
> > > And using:
> > >   make oldconfig < /dev/null
> > >
> > > This was discovered when working on Yocto bug[0] on a downstream
> > > kconfig user (U-boot)
> > >
> > > [0]: https://bugzilla.yoctoproject.org/show_bug.cgi?id=14136
> > >
> > > CC: Brandon Maier <brandon.maier@collins.com>
> > > Signed-off-by: Yoann Congal <yoann.congal@smile.fr>
> >
> > Applied to linux-kbuild.
> > Thanks.
>
> For what it's worth, this change breaks our continuous integration [1]
> because tuxmake explicitly uses /dev/null as stdin to make for
> non-interactive commands (I think it does this as basically the
> equivalent of "yes '' | make" in Python), so the error will always
> occur.
>
> Before:
>
> $ curl -LSso .config https://gitlab.archlinux.org/archlinux/packaging/packages/linux/-/raw/main/config
>
> # Same as 'make < /dev/null' but still
> $ python3 -c "import subprocess; subprocess.run(['make', '-j128'], stdin=subprocess.DEVNULL)"
>   SYNC    include/config/auto.conf
>   HOSTCC  scripts/basic/fixdep
>   HOSTCC  scripts/kconfig/conf.o
>   HOSTCC  scripts/kconfig/confdata.o
>   HOSTCC  scripts/kconfig/expr.o
>   LEX     scripts/kconfig/lexer.lex.c
>   YACC    scripts/kconfig/parser.tab.[ch]
>   HOSTCC  scripts/kconfig/menu.o
>   HOSTCC  scripts/kconfig/preprocess.o
>   HOSTCC  scripts/kconfig/symbol.o
>   HOSTCC  scripts/kconfig/util.o
>   HOSTCC  scripts/kconfig/lexer.lex.o
>   HOSTCC  scripts/kconfig/parser.tab.o
>   HOSTLD  scripts/kconfig/conf
> *
> * Restart config...
> *
> ...
> Error in reading or end of file.
>
>   GEN     arch/x86/include/generated/asm/orc_hash.h
>   WRAP    arch/x86/include/generated/uapi/asm/bpf_perf_event.h
>   WRAP    arch/x86/include/generated/uapi/asm/errno.h
> ...
>
> After:
>
> $ curl -LSso .config https://gitlab.archlinux.org/archlinux/packaging/packages/linux/-/raw/main/config
>
> $ python3 -c "import subprocess; subprocess.run(['make', '-j128'], stdin=subprocess.DEVNULL)"
>   SYNC    include/config/auto.conf
>   HOSTCC  scripts/basic/fixdep
>   HOSTCC  scripts/kconfig/conf.o
>   HOSTCC  scripts/kconfig/confdata.o
>   HOSTCC  scripts/kconfig/expr.o
>   LEX     scripts/kconfig/lexer.lex.c
>   YACC    scripts/kconfig/parser.tab.[ch]
>   HOSTCC  scripts/kconfig/menu.o
>   HOSTCC  scripts/kconfig/preprocess.o
>   HOSTCC  scripts/kconfig/symbol.o
>   HOSTCC  scripts/kconfig/util.o
>   HOSTCC  scripts/kconfig/lexer.lex.o
>   HOSTCC  scripts/kconfig/parser.tab.o
>   HOSTLD  scripts/kconfig/conf
> *
> * Restart config...
> *
> ...
> Error in reading or end of file.
> make[3]: *** [scripts/kconfig/Makefile:77: syncconfig] Error 1
> ...
>
> We have been doing this for some time and never run across an infinite
> loop in syncconfig. Can this be improved?



In Linux, most int/hex entries have a default,
hence there is no practical issue.

I will drop this for now.

I will send an alternative solution.








--
Best Regards
Masahiro Yamada

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

* Re: [PATCH v5] kconfig: avoid an infinite loop in oldconfig/syncconfig
  2023-11-09  4:26     ` Masahiro Yamada
@ 2023-11-09 22:56       ` Yoann Congal
  0 siblings, 0 replies; 5+ messages in thread
From: Yoann Congal @ 2023-11-09 22:56 UTC (permalink / raw)
  To: Masahiro Yamada, Nathan Chancellor
  Cc: linux-kbuild, linux-kernel, Randy Dunlap, Brandon Maier,
	Nick Desaulniers, llvm

Hi,

Le 09/11/2023 à 05:26, Masahiro Yamada a écrit :
> On Tue, Nov 7, 2023 at 11:00 PM Nathan Chancellor <nathan@kernel.org> wrote:
>> For what it's worth, this change breaks our continuous integration [1>> because tuxmake explicitly uses /dev/null as stdin to make for
>> non-interactive commands (I think it does this as basically the
>> equivalent of "yes '' | make" in Python), so the error will always
>> occur.
>>
>> Before:
>>
>> [...]
>>
>> After:
>>
>> $ curl -LSso .config https://gitlab.archlinux.org/archlinux/packaging/packages/linux/-/raw/main/config

Calling 'make olddefconfig' at this point should avoid opening the prompt on /dev/null in the next make.

I got tuxmake to that with a hack:
$ .../tuxmake/run --kconfig /dev/null --kconfig-add https://gitlab.archlinux.org/archlinux/packaging/packages/linux/-/raw/main/config
# --kconfig /dev/null      => Start from a blank config
# --kconfig-add https//... => ... add to it a partial config
=> tuxmake will "merge" the empty config and the URL one and then run 'make olddefconfig' and finally 'make < /dev/null' which does run

>> $ python3 -c "import subprocess; subprocess.run(['make', '-j128'], stdin=subprocess.DEVNULL)"
>>   SYNC    include/config/auto.conf
>>   HOSTCC  scripts/basic/fixdep
>>   HOSTCC  scripts/kconfig/conf.o
>>   HOSTCC  scripts/kconfig/confdata.o
>>   HOSTCC  scripts/kconfig/expr.o
>>   LEX     scripts/kconfig/lexer.lex.c
>>   YACC    scripts/kconfig/parser.tab.[ch]
>>   HOSTCC  scripts/kconfig/menu.o
>>   HOSTCC  scripts/kconfig/preprocess.o
>>   HOSTCC  scripts/kconfig/symbol.o
>>   HOSTCC  scripts/kconfig/util.o
>>   HOSTCC  scripts/kconfig/lexer.lex.o
>>   HOSTCC  scripts/kconfig/parser.tab.o
>>   HOSTLD  scripts/kconfig/conf
>> *
>> * Restart config...
>> *
>> ...
>> Error in reading or end of file.
>> make[3]: *** [scripts/kconfig/Makefile:77: syncconfig] Error 1
>> ...
>>
>> We have been doing this for some time and never run across an infinite
>> loop in syncconfig. Can this be improved?
> 
> In Linux, most int/hex entries have a default,
> hence there is no practical issue.

I agree. I never met such case in Linux but only on downstream kbuild user (u-boot in this case).

> I will drop this for now.

Okay!

> I will send an alternative solution.

Please tell me how I can help!

For what it's worth the v2 of this patch[0] tried to exit *only* where the infinite loop would start.
I've just tested it, it allows tuxmake to run smoothly and avoids the infinite loop in case of a hex config without a valid default value.

[0]:https://lore.kernel.org/lkml/20230805095709.6717-1-yoann.congal@smile.fr/

Regards,
-- 
Yoann Congal
Smile ECS

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

end of thread, other threads:[~2023-11-09 22:56 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-04 22:27 [PATCH v5] kconfig: avoid an infinite loop in oldconfig/syncconfig Yoann Congal
2023-11-05  7:55 ` Masahiro Yamada
2023-11-07 21:00   ` Nathan Chancellor
2023-11-09  4:26     ` Masahiro Yamada
2023-11-09 22:56       ` Yoann Congal

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).