| Message ID | 20231031111647.111093-1-yoann.congal@smile.fr (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v4] kconfig: avoid an infinite loop in oldconfig/syncconfig | expand |
On Tue, Oct 31, 2023 at 8:17 PM 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 > > Signed-off-by: Yoann Congal <yoann.congal@smile.fr> > Tested-by: Brandon Maier <brandon.maier@collins.com> > --- > 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 | 19 +++++++++++++++---- > 1 file changed, 15 insertions(+), 4 deletions(-) > > diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c > index 33d19e419908..68f0c649a805 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; > } This is not what I suggested. I suggested much simpler code: https://lore.kernel.org/linux-kbuild/CAK7LNAS8a=8n9r7kQrLTPpKwqXG1d1sd0WjJ8PQhOXHXxnSyNQ@mail.gmail.com/ It is easy to know the symbol name that is causing the issue; it was shown in the prompt just before the failure. Also, please note that the 'x' prefix is often used for functions that do not require error check. (e.g. xmalloc) > > 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 33d19e419908..68f0c649a805 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> Tested-by: Brandon Maier <brandon.maier@collins.com> --- 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 | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-)