Skip to content

fix(menuconfig): allow choice override menuconfig #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 20, 2023

Conversation

MrKevinWeiss
Copy link

Hi 👋

I have encountered this issue when spending far too much time with kconfig and RIOT...

Handle special cases when having a choice override with a menuconfig.

This is a bit more complete solution to ulfalizer#94

The failure can be checked with:

config PRE
    bool "Pre prompt"

choice OVERRIDE_ME

menuconfig PRE_MENUCONFIG
    bool "PRE_MENUCONFIG prompt"

config PRE_CONFIG
    bool "PRE_CONFIG prompt"
    depends on PRE_MENUCONFIG

endchoice

menuconfig FOO
    bool "FOO prompt"

if FOO

config FOOPRE
    bool "FOOPRE prompt"

choice OVERRIDE_ME
    bool "OVERRIDE_ME prompt"

menuconfig DOES_NOT_CRASH_MENUCONFIG
    bool "DOES_NOT_CRASH_MENUCONFIG prompt"

config DOES_NOT_CRASH_CONFIG
    bool "DOES_NOT_CRASH_CONFIG prompt"
    depends on DOES_NOT_CRASH_MENUCONFIG

endchoice

config FOOPOST
    bool "FOOPOST prompt"

endif

choice OVERRIDE_ME

menuconfig POST_MENUCONFIG
    bool "POST_MENUCONFIG prompt"

config POST_CONFIG
    bool "POST_CONFIG prompt"
    depends on POST_MENUCONFIG

endchoice

config POST
    bool "POST prompt"

Here is the failure and solution in action...
Peek 2023-01-03 12-42

Handle special cases when having a choice override with a menuconfig.
@MrKevinWeiss
Copy link
Author

ping @leandrolanzieri just to do a quick check...

Copy link

@leandrolanzieri leandrolanzieri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK. Tested with the provided Kconfig file and indeed it fixes the crash.

@leandrolanzieri leandrolanzieri merged commit 110688d into RIOT-OS:master Jan 20, 2023
@leandrolanzieri leandrolanzieri added the bug Something isn't working label Jan 20, 2023
@MrKevinWeiss
Copy link
Author

Note that this case only trigger if there is a failure, if there is a missing corner-case then the selected node will just be messed up, maybe we could have set _sel_node_i to 0 instead of pass but we will see if anything pops up.

@MrKevinWeiss MrKevinWeiss deleted the pr/menuconfigfix branch January 20, 2023 08:55
@MrKevinWeiss
Copy link
Author

Thanks @leandrolanzieri and @kaspar030

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants