Skip to contentSkip to conversation

kconfig/lxdialog: fix check() with GCC14

PATCH Superseded Robert Marko 4 participants 9 messages Apr 3, 2024 – Apr 11, 2024

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 kconfig/lxdialog: fix check() with GCC14

GCC14 now treats implicit int types as error so when check() from check-lxdialog.sh is called to check whether we can link against ncurses it will fail silently and the help text indicating to install ncurses is printed.

However, this is not due to missing ncurses but once the stderr redirect to /dev/null is removed we can see the root cause:
<stdin>:2:1: error: return type defaults to ‘int’ [-Wimplicit-int]

So, in order for menuconfig to work with GCC14 lets just specify the return type of main() as int.

---
 support/kconfig/lxdialog/check-lxdialog.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
support/kconfig/lxdialog/check-lxdialog.sh +1 -1
index 16cd9a3186..27d6c30a57 100755
@@ -48,7 +48,7 @@ trap "rm -f $tmp" 0 1 2 3 15
 check() {
         $cc -x c - -o $tmp 2>/dev/null <<'EOF'
 #include CURSES_LOC
-main() {}
+int main() {}
 EOF
 	if [ $? != 0 ]; then
 	    echo " *** Unable to find the ncurses libraries or the"       1>&2
Signature · 6 lines
2.44.0

_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot
Reviewed-by Tested-by on v1 1/1
#1 lore raw

Hi Robert,

Quoted 11 lines

GCC14 now treats implicit int types as error so when check() from check-lxdialog.sh is called to check whether we can link against ncurses it will fail silently and the help text indicating to install ncurses is printed.

However, this is not due to missing ncurses but once the stderr redirect to /dev/null is removed we can see the root cause:
<stdin>:2:1: error: return type defaults to ‘int’ [-Wimplicit-int]

So, in order for menuconfig to work with GCC14 lets just specify the return type of main() as int.

Good catch. Not only gcc-14, but also clang-16 fails due -Wimplicit-int. (clang-14 was ok).

Kind regards,
Petr

Quoted 10 lines
---
 support/kconfig/lxdialog/check-lxdialog.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
support/kconfig/lxdialog/check-lxdialog.sh +0 -0
index 16cd9a3186..27d6c30a57 100755

support/kconfig/lxdialog/check-lxdialog.sh trap "rm -f $tmp" 0 1 2 3 15 · line 54

Quoted 8 lines
support/kconfig/lxdialog/check-lxdialog.sh +1 -1
@@ -48,7 +48,7 @@ trap "rm -f $tmp" 0 1 2 3 15
 check() {
         $cc -x c - -o $tmp 2>/dev/null <<'EOF'
 #include CURSES_LOC
-main() {}
+int main() {}
 EOF
 	if [ $? != 0 ]; then
 	    echo " *** Unable to find the ncurses libraries or the"       1>&2

_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot

#2 lore raw
In reply to Robert Marko

Hello Robert,

Quoted 5 lines · On Wed, 3 Apr 2024 14:18:07 +0200 Robert Marko <[email protected]> wrote:

GCC14 now treats implicit int types as error so when check() from check-lxdialog.sh is called to check whether we can link against ncurses it will fail silently and the help text indicating to install ncurses is printed.

Quoted 8 lines

However, this is not due to missing ncurses but once the stderr redirect to /dev/null is removed we can see the root cause:
<stdin>:2:1: error: return type defaults to ‘int’ [-Wimplicit-int]

So, in order for menuconfig to work with GCC14 lets just specify the return type of main() as int.

Thanks a lot for the patch. However, making changes in support/kconfig/ in Buildroot requires a bit more care. Indeed, this code is taken from the Linux kernel code, and we try to keep a well-defined list of changes on top of the original code from the kernel.

As explained in support/kconfig/README.buildroot, the current code in support/kconfig/ is taken from Linux 4.17-rc2, on top of which the patches in support/kconfig/patches/ have been applied.

So from this, you have two options:

(1) Add a new patch in support/kconfig/patches/ which implements the change you need for GCC14 compatibility

(2) Update the code in support/kconfig/ to the latest code available in the Linux kernel scripts/kconfig directory, rebase our patches, and verify it all works.

Obviously, (1) is the easiest path. (2) is nicer long term, as we benefit from all new features/fixes added by the Linux kernel developers. Turns out that in the upstream kernel, the check-lxdialog.sh script has been removed and replaced by something different, which no longer has this main() function definition.

Could you have a look at implementing either (1) or (2) ?

Thanks a lot!

Thomas

Signature · 7 lines
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot
#3 lore raw
Quoted 38 lines · Thomas Petazzoni

Hello Robert,

Quoted 5 lines · On Wed, 3 Apr 2024 14:18:07 +0200 Robert Marko <[email protected]> wrote:

GCC14 now treats implicit int types as error so when check() from check-lxdialog.sh is called to check whether we can link against ncurses it will fail silently and the help text indicating to install ncurses is printed.

Quoted 8 lines

However, this is not due to missing ncurses but once the stderr redirect to /dev/null is removed we can see the root cause:
<stdin>:2:1: error: return type defaults to ‘int’ [-Wimplicit-int]

So, in order for menuconfig to work with GCC14 lets just specify the return type of main() as int.

Thanks a lot for the patch. However, making changes in support/kconfig/ in Buildroot requires a bit more care. Indeed, this code is taken from the Linux kernel code, and we try to keep a well-defined list of changes on top of the original code from the kernel.

As explained in support/kconfig/README.buildroot, the current code in support/kconfig/ is taken from Linux 4.17-rc2, on top of which the patches in support/kconfig/patches/ have been applied.

So from this, you have two options:

(1) Add a new patch in support/kconfig/patches/ which implements the change you need for GCC14 compatibility

(2) Update the code in support/kconfig/ to the latest code available in the Linux kernel scripts/kconfig directory, rebase our patches, and verify it all works.

Quoted 7 lines

Obviously, (1) is the easiest path. (2) is nicer long term, as we benefit from all new features/fixes added by the Linux kernel developers. Turns out that in the upstream kernel, the check-lxdialog.sh script has been removed and replaced by something different, which no longer has this main() function definition.

Could you have a look at implementing either (1) or (2) ?

Hi Thomas,
I can only implement solution 1 currently, will respin for v2.

Regards,
Robert

Quoted 8 lines

Thanks a lot!

Thomas
--
Thomas Petazzoni, co-owner and CEO, Bootlin Embedded Linux and Kernel engineering and training
https://bootlin.com

_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot

#4 lore raw
In reply to Thomas Petazzoni via buildroot

Hi Robert, Thomas,

Quoted 19 lines

Hello Robert,

Quoted 5 lines · On Wed, 3 Apr 2024 14:18:07 +0200 Robert Marko <[email protected]> wrote:

GCC14 now treats implicit int types as error so when check() from check-lxdialog.sh is called to check whether we can link against ncurses it will fail silently and the help text indicating to install ncurses is printed.

Quoted 8 lines

However, this is not due to missing ncurses but once the stderr redirect to /dev/null is removed we can see the root cause:
<stdin>:2:1: error: return type defaults to ‘int’ [-Wimplicit-int]

So, in order for menuconfig to work with GCC14 lets just specify the return type of main() as int.

Quoted 8 lines

Thanks a lot for the patch. However, making changes in support/kconfig/ in Buildroot requires a bit more care. Indeed, this code is taken from the Linux kernel code, and we try to keep a well-defined list of changes on top of the original code from the kernel.

As explained in support/kconfig/README.buildroot, the current code in support/kconfig/ is taken from Linux 4.17-rc2, on top of which the patches in support/kconfig/patches/ have been applied.

I'm sorry for a wrong review, I completely forget Buildroot uses patches also for kconfig.

Quoted 9 lines

So from this, you have two options:

(1) Add a new patch in support/kconfig/patches/ which implements the change you need for GCC14 compatibility

(2) Update the code in support/kconfig/ to the latest code available in the Linux kernel scripts/kconfig directory, rebase our patches, and verify it all works.

Quoted 5 lines

Obviously, (1) is the easiest path. (2) is nicer long term, as we benefit from all new features/fixes added by the Linux kernel developers. Turns out that in the upstream kernel, the check-lxdialog.sh script has been removed and replaced by something different, which no longer has this main() function definition.

FYI I gave up kconfig update after big rewrites in the Linux kernel made by Masahiro Yamada (the new kbuild and kconfig maintainer), because the code changes were quite big but kconfig in Buildroot is IMHO not missing any significant usability function. But I can be wrong, Thomas let me know if you miss something significant which would justify the update. Obviously we would get some fixes, but also major rewrite, which would likely require changes in other parts of Buildroot.

check-lxdialog.sh was removed in 1c5af5cf9308 ("kconfig: refactor ncurses package checks for building mconf and nconf") in v4.18, but major rewrite come in 4.19 and 5.0 + kbuild updates in 5.1, changes continue in 5.10, 5.13, later fixes in 6.x kernels...

Some of these changes moved storing toolchain capabilities in .config (while Buildroot uses does something similar, I'm not sure whether kernel functionality could be used without big changes in Buildroot).

Kind regards,
Petr

Quoted 5 lines

Could you have a look at implementing either (1) or (2) ?

Thanks a lot!

Thomas

_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot

#5 lore raw
Quoted 7 lines · On Mon, 8 Apr 2024 23:38:07 +0200 Petr Vorel <[email protected]> wrote:

FYI I gave up kconfig update after big rewrites in the Linux kernel made by Masahiro Yamada (the new kbuild and kconfig maintainer), because the code changes were quite big but kconfig in Buildroot is IMHO not missing any significant usability function. But I can be wrong, Thomas let me know if you miss something significant which would justify the update. Obviously we would get some fixes, but also major rewrite, which would likely require changes in other parts of Buildroot.

I have no idea what are the new interesting features that have been added in the upstream scripts/kconfig/ (if any). However, keeping our support/kconfig/ reasonably up-to-date is not only about new features, it's also about staying compatible with reasonably recent libraries/tools (more recent ncurses, Gtk, Qt, more recent GCC, like this issue illustrates).

Best regards,

Thomas

Signature · 7 lines
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot
#6 lore raw

Thomas, Petr, All,

On 2024-04-09 21:46 +0200, Thomas Petazzoni via buildroot spake thusly:

Quoted 11 lines
Quoted 7 lines · On Mon, 8 Apr 2024 23:38:07 +0200 Petr Vorel <[email protected]> wrote:

FYI I gave up kconfig update after big rewrites in the Linux kernel made by Masahiro Yamada (the new kbuild and kconfig maintainer), because the code changes were quite big but kconfig in Buildroot is IMHO not missing any significant usability function. But I can be wrong, Thomas let me know if you miss something significant which would justify the update. Obviously we would get some fixes, but also major rewrite, which would likely require changes in other parts of Buildroot.

Quoted 6 lines

I have no idea what are the new interesting features that have been added in the upstream scripts/kconfig/ (if any). However, keeping our support/kconfig/ reasonably up-to-date is not only about new features, it's also about staying compatible with reasonably recent libraries/tools (more recent ncurses, Gtk, Qt, more recent GCC, like this issue illustrates).

The kernel's kconfig has dropped the possibility to include files from within a choice, so what we're doing now in a few places will not work;
- toolchain/toolchain-external/Config.in
- package/openssl/Config.in

and so on...

Unless that was reverted upstream...

Regards,
Yann E. MORIN.

Signature · 10 lines
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot
#7 lore raw

Hi Yann, Thomas, all,

Quoted 21 lines

Thomas, Petr, All,

On 2024-04-09 21:46 +0200, Thomas Petazzoni via buildroot spake thusly:

Quoted 11 lines
Quoted 7 lines · On Mon, 8 Apr 2024 23:38:07 +0200 Petr Vorel <[email protected]> wrote:

FYI I gave up kconfig update after big rewrites in the Linux kernel made by Masahiro Yamada (the new kbuild and kconfig maintainer), because the code changes were quite big but kconfig in Buildroot is IMHO not missing any significant usability function. But I can be wrong, Thomas let me know if you miss something significant which would justify the update. Obviously we would get some fixes, but also major rewrite, which would likely require changes in other parts of Buildroot.

Quoted 6 lines

I have no idea what are the new interesting features that have been added in the upstream scripts/kconfig/ (if any). However, keeping our support/kconfig/ reasonably up-to-date is not only about new features, it's also about staying compatible with reasonably recent libraries/tools (more recent ncurses, Gtk, Qt, more recent GCC, like this issue illustrates).

Quoted 4 lines

The kernel's kconfig has dropped the possibility to include files from within a choice, so what we're doing now in a few places will not work;
- toolchain/toolchain-external/Config.in
- package/openssl/Config.in

Ah, you mean choice within choice .. endchoice. Thanks for info. That's unfortunate. Any idea which commits reverted it? I'm not sure if it will be possible to easily revert it back (there are many changes in kconfig). Also replacing 'source' with some placeholder and using some preprocessing in extra make target (dependency for gconfig/xconfig/menuconfig/nconfig/config) might not be as trivial as it looks like.

Quoted 1 line

and so on...

Do you remember any other problem?

Quoted 1 line

Unless that was reverted upstream...

I doubt, but that needs to be find out.

Also, maybe slightly easier way could be to use init-kconfig [1], used in kdevops [2] (imported via 'git subtree' into the usual scripts/kconfig/ directory). At least we could try to migrate at least some of our patches there.

Kind regards,
Petr

Quoted 2 lines

Regards,
Yann E. MORIN.

[1] https://github.com/mcgrof/init-kconfig
[2] https://github.com/mcgrof/kdevops
_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot

#8 lore raw

Petr, All,

Quoted 5 lines
Quoted 4 lines

The kernel's kconfig has dropped the possibility to include files from within a choice, so what we're doing now in a few places will not work;
- toolchain/toolchain-external/Config.in
- package/openssl/Config.in

Ah, you mean choice within choice .. endchoice. Thanks for info. That's

               ^^^^^^

s/choice/source/

Yes, "source" in a choice...endchoice block.

Quoted 1 line

unfortunate. Any idea which commits reverted it?

That was commit in the kernel, 4 years ago:

    09d5873e4d1f7 kconfig: allow only 'config', 'comment', and 'if' inside 'choice'
Quoted 2 lines

I'm not sure if it will be
possible to easily revert it back (there are many changes in kconfig).

Indeed.

Quoted 3 lines

Also replacing 'source' with some placeholder and using some preprocessing in extra make target (dependency for gconfig/xconfig/menuconfig/nconfig/config) might not be as trivial as it looks like.

No, it is not...

Quoted 1 line

Do you remember any other problem?

Not right now. There were a few other major changes, but AFAICS we would not be impacted.

Quoted 1 line

Also, maybe slightly easier way could be to use init-kconfig [1],

init-kconfig has not been touched in 2 years. There is no point in switching from our mothballed copy, to another mothballed copy...

And in any case, they advertise as being "a passive fork of Linux kconfig", and their "goal is not to fork kconfig and evolve it separately".

Regards,
Yann E. MORIN.

Signature · 10 lines
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot