* [PATCH v3] kconfig: avoid an infinite loop in oldconfig/syncconfig
@ 2023-09-12 15:48 Yoann Congal
2023-10-04 21:52 ` Brandon Maier
0 siblings, 1 reply; 3+ messages in thread
From: Yoann Congal @ 2023-09-12 15:48 UTC (permalink / raw)
To: Masahiro Yamada, linux-kbuild; +Cc: linux-kernel, Randy Dunlap, Yoann Congal
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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v3] kconfig: avoid an infinite loop in oldconfig/syncconfig
2023-09-12 15:48 [PATCH v3] kconfig: avoid an infinite loop in oldconfig/syncconfig Yoann Congal
@ 2023-10-04 21:52 ` Brandon Maier
2023-10-04 22:47 ` Yoann Congal
0 siblings, 1 reply; 3+ messages in thread
From: Brandon Maier @ 2023-10-04 21:52 UTC (permalink / raw)
To: Yoann Congal; +Cc: Masahiro Yamada, linux-kbuild, linux-kernel, Randy Dunlap
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
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v3] kconfig: avoid an infinite loop in oldconfig/syncconfig
2023-10-04 21:52 ` Brandon Maier
@ 2023-10-04 22:47 ` Yoann Congal
0 siblings, 0 replies; 3+ messages in thread
From: Yoann Congal @ 2023-10-04 22:47 UTC (permalink / raw)
To: Brandon Maier; +Cc: Masahiro Yamada, linux-kbuild, linux-kernel, Randy Dunlap
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
>>
--
Yoann Congal
Smile ECS - Tech Expert
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-10-04 22:47 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-12 15:48 [PATCH v3] kconfig: avoid an infinite loop in oldconfig/syncconfig Yoann Congal
2023-10-04 21:52 ` Brandon Maier
2023-10-04 22:47 ` 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).