| Message ID | 20231104222715.3967791-1-yoann.congal@smile.fr (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v5] kconfig: avoid an infinite loop in oldconfig/syncconfig | expand |
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 >
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
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
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,
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);
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(-)