Support non-binary kconfig options in kernel feature source #197
Conversation
k8s-ci-robot
added
approved
marquiz
force-pushed
the
devel/kconfig
branch
from
d37f833 to
1596775
Compare
k8s-ci-robot
added
size/S
source/kernel/kernel.go
Outdated
| kconfig[m[1]] = true | ||
| kconfig[m[1]] = "true" | ||
| } else { | ||
| kconfig[m[1]] = strings.Trim(m[2], `"`) |
There was a problem hiding this comment.
Generally ok, however here it might be good to add a bit more validation for value, beside just stripping "
There was a problem hiding this comment.
Validation of label values implemented in #199. I think that should be enough, WYDT?
There was a problem hiding this comment.
in theory should be enough. my comment was more about general practice of validating input close to the point of source of information.
There was a problem hiding this comment.
I wouldn't want to do duplicate regexp matching in multiple places. Checking the length makes sense so I added that.
Discover other than bool or tristate kconfig options, too. For bool and tristate the node label is still binary (i.e. set to "true" if the kconfig option has been enabled). For other kconfig types (e.g. string or int) the value of the label directly corresponds to the value of the kconfig flag, e.g. "32", "elf64-x86-64" etc.
marquiz
force-pushed
the
devel/kconfig
branch
from
1596775 to
eeed6d1
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: marquiz The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Issues go stale after 90d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
k8s-ci-robot
added
the
lifecycle/stale
|
/lgtm |
k8s-ci-robot
added
the
lgtm
Fix a build failure that slipped through when adding support for non-binary kconfig options in kubernetes-sigs#197,
Discover other than bool or tristate kconfig options, too. For bool and tristate the node label is still binary (i.e. set to "true" if the kconfig option has been enabled). For other kconfig types (e.g. string or int) the value of the label directly corresponds to the value of the kconfig flag, e.g. "32", "elf64-x86-64" etc.