Skip to content
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

kconfig: The error message is misleading when values are out-of-range #6749

Closed
SebastianBoe opened this issue Mar 22, 2018 · 9 comments

Comments

Projects
None yet
3 participants
@SebastianBoe
Copy link
Collaborator

commented Mar 22, 2018

Hi,

the error message is misleading when values are out-of-range. E.g.

CONFIG_FOO=5

 config FOO
     range 0 3

Should trigger an error about the value being out-of-range, but instead it triggers a warning about dependencies.

warning: FOO was assigned the value "5" but got the value "0" -- check dependencies

@ulfalizer

This comment has been minimized.

Copy link
Collaborator

commented Mar 22, 2018

Might be tricky to infer that it was specifically due to the range from kconfig.py. Maybe something like this would point people in the right direction though:

msg = "warning: {} was assigned..."
if sym.ranges:
    msg += " and range constraints"
print(msg, ...)

The behavior of range is a bit wonky in general. default 5 is clamped to 3, while 5 as a user value causes the lower end of the range to be used. It's only done like that for compatibility with the C implementation. It would make more sense to clamp both to 3.

@ulfalizer

This comment has been minimized.

Copy link
Collaborator

commented Mar 22, 2018

I tweaked https://github.com/ulfalizer/Kconfiglib/blob/master/examples/merge_config.py to also print the location of the symbol ("FOO (defined at bar/Kconfig:73) was assigned...") btw. That might be nice to have in the Zephyr version too.

@SebastianBoe

This comment has been minimized.

Copy link
Collaborator Author

commented Mar 22, 2018

The behavior of range is a bit wonky in general. default 5 is clamped to 3, while 5 as a user value causes the lower end of the range to be used. It's only done like that for compatibility with the C implementation. It would make more sense to clamp both to 3.

I'd say it would make more sense to error-out than to do any clamping.

The docs don't mention anything about clamping, but instead state that the user must
stay within the range. Which implies to me that an error should be triggered.

  • numerical ranges: "range" ["if" ]
    This allows to limit the range of possible input values for int
    and hex symbols. The user can only input a value which is larger than
    or equal to the first symbol and smaller than or equal to the second
    symbol.
@SebastianBoe

This comment has been minimized.

Copy link
Collaborator Author

commented Mar 22, 2018

I am happy with any solution that makes the user realize he is out-of-range.

@SebastianBoe

This comment has been minimized.

Copy link
Collaborator Author

commented Mar 22, 2018

msg = "warning: {} was assigned..."
if sym.ranges:
    msg += " and range constraints"
print(msg, ...)

This should work.

@ulfalizer

This comment has been minimized.

Copy link
Collaborator

commented Mar 22, 2018

A warning from Kconfiglib might be nice at least, when it detects that the user value is out-of-range. Should check that it doesn't get super spammy for the Linux kernel configs though.

Should get back to finding an accountant now. Code is more fun than IRL...

@ulfalizer

This comment has been minimized.

Copy link
Collaborator

commented Mar 23, 2018

I added a warning for it: ulfalizer/Kconfiglib@7ba79ca

@carlescufi carlescufi added this to the v1.12.0 milestone Mar 25, 2018

@SebastianBoe

This comment has been minimized.

Copy link
Collaborator Author

commented Mar 26, 2018

@ulfalizer : Much better, I would prefer an error, but if there are unresolvable compatibility issues with Linux then a warning is good enough.

ulfalizer added a commit to ulfalizer/zephyr that referenced this issue Mar 28, 2018

kconfiglib: Update to 7245bad9ebb58
Update Kconfiglib to upstream revision 7245bad9ebb58 (+ local Zephyr
modifications), to get the following improvements/fixes in:

 - Print a warning if an int or hex symbol is assigned a value that lies
   outside an active 'range' constraint.

 - Turn 'FOO' into a link in 'select FOO' when generating the Kconfig
   reference documentation.

 - Parenthesize '&&' expressions within '||' expressions --
   (A && B) || (C && D) is more readable than A && B || C && D.

Fixes zephyrproject-rtos#6749
Fixes zephyrproject-rtos#6844

Origin: https://github.com/zephyrproject-rtos/Kconfiglib/tree/zephyr

Signed-off-by: Ulf Magnusson <ulfalizer@gmail.com>

ulfalizer added a commit to ulfalizer/zephyr that referenced this issue Mar 28, 2018

kconfiglib: Update to 7245bad9ebb58
Update Kconfiglib to upstream revision 7245bad9ebb58 (+ local Zephyr
modifications), to get the following improvements/fixes in:

 - Print a warning if an int or hex symbol is assigned a value that lies
   outside an active 'range' constraint.

 - Turn 'FOO' into a link in 'select FOO' when generating the Kconfig
   reference documentation.

 - Parenthesize '&&' expressions within '||' expressions --
   (A && B) || (C && D) is more readable than A && B || C && D.

The final two fixes will only be visible once the fix for zephyrproject-rtos#5622 gets in.

Fixes zephyrproject-rtos#6749
Fixes zephyrproject-rtos#6844

Origin: https://github.com/zephyrproject-rtos/Kconfiglib/tree/zephyr

Signed-off-by: Ulf Magnusson <ulfalizer@gmail.com>

carlescufi added a commit that referenced this issue Mar 28, 2018

kconfiglib: Update to 7245bad9ebb58
Update Kconfiglib to upstream revision 7245bad9ebb58 (+ local Zephyr
modifications), to get the following improvements/fixes in:

 - Print a warning if an int or hex symbol is assigned a value that lies
   outside an active 'range' constraint.

 - Turn 'FOO' into a link in 'select FOO' when generating the Kconfig
   reference documentation.

 - Parenthesize '&&' expressions within '||' expressions --
   (A && B) || (C && D) is more readable than A && B || C && D.

The final two fixes will only be visible once the fix for #5622 gets in.

Fixes #6749
Fixes #6844

Origin: https://github.com/zephyrproject-rtos/Kconfiglib/tree/zephyr

Signed-off-by: Ulf Magnusson <ulfalizer@gmail.com>

GiulianoFranchetto added a commit to GiulianoFranchetto/zephyr that referenced this issue Apr 9, 2018

kconfiglib: Update to 7245bad9ebb58
Update Kconfiglib to upstream revision 7245bad9ebb58 (+ local Zephyr
modifications), to get the following improvements/fixes in:

 - Print a warning if an int or hex symbol is assigned a value that lies
   outside an active 'range' constraint.

 - Turn 'FOO' into a link in 'select FOO' when generating the Kconfig
   reference documentation.

 - Parenthesize '&&' expressions within '||' expressions --
   (A && B) || (C && D) is more readable than A && B || C && D.

The final two fixes will only be visible once the fix for zephyrproject-rtos#5622 gets in.

Fixes zephyrproject-rtos#6749
Fixes zephyrproject-rtos#6844

Origin: https://github.com/zephyrproject-rtos/Kconfiglib/tree/zephyr

Signed-off-by: Ulf Magnusson <ulfalizer@gmail.com>
@SebastianBoe

This comment has been minimized.

Copy link
Collaborator Author

commented Jun 14, 2018

I was just now prevented from using an invalid configuration due to this erroring-out at Kconfig.

Great job!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.