| Message ID | 20230912154811.1338390-1-yoann.congal@smile.fr (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] kconfig: avoid an infinite loop in oldconfig/syncconfig | expand |
Hi Yoann, We encountered the same issue in U-Boot under Buildroot. I verified that this patch fixes the issue, causing Kconfig to exit out instead of infinitely looping. Thanks for the fix. Tested-by: Brandon Maier <brandon.maier@collins.com> On Tue, Sep 12, 2023 at 05:48:11PM +0200, Yoann Congal 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 > > Signed-off-by: Yoann Congal <yoann.congal@smile.fr> > --- > 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 | 19 +++++++++++++++---- > 1 file changed, 15 insertions(+), 4 deletions(-) > > diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c > index 33d19e419908b..68f0c649a805e 100644 > --- a/scripts/kconfig/conf.c > +++ b/scripts/kconfig/conf.c > @@ -74,13 +74,17 @@ static void strip(char *str) > } > > /* Helper function to facilitate fgets() by Jean Sacren. */ > -static void xfgets(char *str, int size, FILE *in) > +static int xfgets(char *str, int size, FILE *in) > { > + int ret = 0; > + > if (!fgets(str, size, in)) > - fprintf(stderr, "\nError in reading or end of file.\n"); > + ret = -1; > > if (!tty_stdio) > printf("%s", str); > + > + return ret; > } > > static void set_randconfig_seed(void) > @@ -339,7 +343,10 @@ static int conf_askvalue(struct symbol *sym, const char *def) > /* fall through */ > default: > fflush(stdout); > - xfgets(line, sizeof(line), stdin); > + if (xfgets(line, sizeof(line), stdin) != 0) { > + fprintf(stderr, "Error while reading value of symbol \"%s\"\n", sym->name); > + exit(1); > + } > break; > } > > @@ -521,7 +528,11 @@ static int conf_choice(struct menu *menu) > /* fall through */ > case oldaskconfig: > fflush(stdout); > - xfgets(line, sizeof(line), stdin); > + if (xfgets(line, sizeof(line), stdin) != 0) { > + fprintf(stderr, "Error while reading value of symbol \"%s\"\n", > + sym->name); > + exit(1); > + } > strip(line); > if (line[0] == '?') { > print_help(menu); > -- > 2.30.2 >
On 10/4/23 23:52, Brandon Maier wrote: > Hi Yoann, Hi, > We encountered the same issue in U-Boot under Buildroot. I verified that > this patch fixes the issue, causing Kconfig to exit out instead of > infinitely looping. Thanks for the fix. Awesome! I've also found this on U-boot (but under Yocto). I planned to get this merged here in kbuild then U-boot downstream and then Yocto. Please CC me if you try to do something with this patch in Buildroot :) > Tested-by: Brandon Maier <brandon.maier@collins.com> Thanks! > On Tue, Sep 12, 2023 at 05:48:11PM +0200, Yoann Congal 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 >> >> Signed-off-by: Yoann Congal <yoann.congal@smile.fr> >> --- >> 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 | 19 +++++++++++++++---- >> 1 file changed, 15 insertions(+), 4 deletions(-) >> >> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c >> index 33d19e419908b..68f0c649a805e 100644 >> --- a/scripts/kconfig/conf.c >> +++ b/scripts/kconfig/conf.c >> @@ -74,13 +74,17 @@ static void strip(char *str) >> } >> >> /* Helper function to facilitate fgets() by Jean Sacren. */ >> -static void xfgets(char *str, int size, FILE *in) >> +static int xfgets(char *str, int size, FILE *in) >> { >> + int ret = 0; >> + >> if (!fgets(str, size, in)) >> - fprintf(stderr, "\nError in reading or end of file.\n"); >> + ret = -1; >> >> if (!tty_stdio) >> printf("%s", str); >> + >> + return ret; >> } >> >> static void set_randconfig_seed(void) >> @@ -339,7 +343,10 @@ static int conf_askvalue(struct symbol *sym, const char *def) >> /* fall through */ >> default: >> fflush(stdout); >> - xfgets(line, sizeof(line), stdin); >> + if (xfgets(line, sizeof(line), stdin) != 0) { >> + fprintf(stderr, "Error while reading value of symbol \"%s\"\n", sym->name); >> + exit(1); >> + } >> break; >> } >> >> @@ -521,7 +528,11 @@ static int conf_choice(struct menu *menu) >> /* fall through */ >> case oldaskconfig: >> fflush(stdout); >> - xfgets(line, sizeof(line), stdin); >> + if (xfgets(line, sizeof(line), stdin) != 0) { >> + fprintf(stderr, "Error while reading value of symbol \"%s\"\n", >> + sym->name); >> + exit(1); >> + } >> strip(line); >> if (line[0] == '?') { >> print_help(menu); >> -- >> 2.30.2 >>
diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c index 33d19e419908b..68f0c649a805e 100644 --- a/scripts/kconfig/conf.c +++ b/scripts/kconfig/conf.c @@ -74,13 +74,17 @@ static void strip(char *str) } /* Helper function to facilitate fgets() by Jean Sacren. */ -static void xfgets(char *str, int size, FILE *in) +static int xfgets(char *str, int size, FILE *in) { + int ret = 0; + if (!fgets(str, size, in)) - fprintf(stderr, "\nError in reading or end of file.\n"); + ret = -1; if (!tty_stdio) printf("%s", str); + + return ret; } static void set_randconfig_seed(void) @@ -339,7 +343,10 @@ static int conf_askvalue(struct symbol *sym, const char *def) /* fall through */ default: fflush(stdout); - xfgets(line, sizeof(line), stdin); + if (xfgets(line, sizeof(line), stdin) != 0) { + fprintf(stderr, "Error while reading value of symbol \"%s\"\n", sym->name); + exit(1); + } break; } @@ -521,7 +528,11 @@ static int conf_choice(struct menu *menu) /* fall through */ case oldaskconfig: fflush(stdout); - xfgets(line, sizeof(line), stdin); + if (xfgets(line, sizeof(line), stdin) != 0) { + fprintf(stderr, "Error while reading value of symbol \"%s\"\n", + sym->name); + exit(1); + } strip(line); if (line[0] == '?') { print_help(menu);
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 Signed-off-by: Yoann Congal <yoann.congal@smile.fr> --- 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 | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-)