kconfig: preserve the final answer when input has no newline
PATCH 1/2 kconfig: preserve the final answer when input has no newline
conf_string() unconditionally removes the last character returned by fgets(), assuming that it is a newline. When redirected input ends without a newline, the last character is part of the answer instead. For example, feeding 42 to an integer prompt stores 4, and feeding 0xff to a hexadecimal prompt stores 0xf. Both commands succeed despite silently changing the supplied value.
Strip the newline with strcspn() so that a complete answer at EOF is preserved. Keep the existing handling of newline-terminated and empty answers unchanged.
---
An AI coding assistant found the issue, prepared the fix and changelog, and ran the verification below after a request to find reproducible functional bugs in Linux development tools.
Validation:
- Built conf with Clang on macOS and GCC in an x86_64 Linux container, using -Wall -Wmissing-prototypes -Wstrict-prototypes -Werror.
- Ran 60 before/after executions on each platform, covering oldaskconfig and oldconfig with string, int and hex answers. Unterminated answers retain their last character. Newline, empty-answer, empty-input and CRLF controls keep the same results.
- The 21 existing Kconfig tests pass on macOS.
- This tests the host configuration tool; a complete kernel was not built.
scripts/kconfig/conf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
scripts/kconfig/conf.c +1 -1
@@ -343,7 +343,7 @@ static int conf_string(struct menu *menu) }/* fall through */ default : - line[strlen (line)-1 ] =0 ; + line[strcspn (line,"\n" )] =0 ; def = line; }if (def &&sym_set_string_value (sym, def))
base-commit: 4d7d9486c04d917265f64c55bd23b2cc4fe7749c
Signature · 1 line
2.50.1 (Apple Git-155)
PATCH 2/2 checkkconfigsymbols: resolve revisions before resetting the tree
The commit comparison resets the current branch to commit_a before resolving commit_b. If the second revision is HEAD or the current branch name, it then resolves to the first revision. For example, --diff HEAD^..HEAD compares the parent with itself and silently misses newly undefined symbols. The same problem affects --commit with the current branch name.
Resolve and verify both revisions as commits before the first reset. This keeps their meaning stable throughout the comparison and rejects invalid endpoints before changing the working tree.
Keep lookup diagnostics separate from the resolved hashes. Use the resolved range for --find too, since resetting also updates ORIG_HEAD.
---
An AI coding assistant found the issue, prepared the fix and changelog, and ran the verification below after a request to find reproducible functional bugs in Linux development tools.
Validation:
- Ran the actual CLI against disposable Git repositories, comparing the original script with the patched source. All 15 patched cases pass.
- Covered HEAD-relative ranges, branch names, hashes, tags, annotated tags, ambiguous branch/tag names, identical revisions, --commit, --find, ORIG_HEAD and invalid or non-commit endpoints.
- Checked HEAD, current branch, index and tracked file contents after every invocation. The patched cases preserve these values, including when the second revision is invalid.
scripts/checkkconfigsymbols.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
scripts/checkkconfigsymbols.py +9 -2
@@ -150,6 +150,13 @@ def print_undefined_symbols(): undefined_a = {} undefined_b = {} + commit_a = execute(["git", "rev-parse", "--verify", commit_a + "^{commit}"], + stderr=None).strip() + commit_b = execute(["git", "rev-parse", "--verify", commit_b + "^{commit}"], + stderr=None).strip() + if args.diff: + args.diff = commit_a + ".." + commit_b + # get undefined items before the commit reset(commit_a) undefined_a, _ = check_symbols(args.ignore)
@@ -223,10 +230,10 @@ def red(string): return "\033[31m%s\033[0m" % string if COLOR else string -def execute(cmd): +def execute(cmd, stderr=subprocess.STDOUT): """Execute %cmd and return stdout. Exit in case of error.""" try: - stdout = subprocess.check_output(cmd, stderr=subprocess.STDOUT, shell=False) + stdout = subprocess.check_output(cmd, stderr=stderr, shell=False) stdout = stdout.decode(errors='replace') except subprocess.CalledProcessError as fail: exit(fail)
Signature · 1 line
2.50.1 (Apple Git-155)
Julian Braha
reviewerRe: [PATCH 2/2] checkkconfigsymbols: resolve revisions before resetting the tree
Quoted 58 lines · Erkan Erdem
The commit comparison resets the current branch to
commit_abefore resolvingcommit_b. If the second revision is HEAD or the current branch name, it then resolves to the first revision. For example, --diff HEAD^..HEAD compares the parent with itself and silently misses newly undefined symbols. The same problem affects --commit with the current branch name.Resolve and verify both revisions as commits before the first reset. This keeps their meaning stable throughout the comparison and rejects invalid endpoints before changing the working tree.
Keep lookup diagnostics separate from the resolved hashes. Use the resolved range for --find too, since resetting also updates
ORIG_HEAD.Fixes b1a3f243485f ("checkkconfigsymbols.py: make it Git aware")Assisted-by LLMSigned-off-by Erkan Erdem <[email protected]>---
An AI coding assistant found the issue, prepared the fix and changelog, and ran the verification below after a request to find reproducible functional bugs in Linux development tools.
Validation:
- Ran the actual CLI against disposable Git repositories, comparing the original script with the patched source. All 15 patched cases pass.
- Covered HEAD-relative ranges, branch names, hashes, tags, annotated tags, ambiguous branch/tag names, identical revisions, --commit, --find,ORIG_HEADand invalid or non-commit endpoints.
- Checked HEAD, current branch, index and tracked file contents after every invocation. The patched cases preserve these values, including when the second revision is invalid.scripts/checkkconfigsymbols.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)scripts/checkkconfigsymbols.py +7
-0@@ -150,6 +150,13 @@ def print_undefined_symbols(): undefined_a = {} undefined_b = {} + commit_a = execute(["git", "rev-parse", "--verify", commit_a + "^{commit}"], + stderr=None).strip() + commit_b = execute(["git", "rev-parse", "--verify", commit_b + "^{commit}"], + stderr=None).strip() + if args.diff: + args.diff = commit_a + ".." + commit_b + # get undefined items before the commit reset(commit_a) undefined_a, _ = check_symbols(args.ignore)@@ -223,10 +230,10 @@ def red(string): return "\033[31m%s\033[0m" % string if COLOR else string
scripts/checkkconfigsymbols.py def red(string): · line 239
Quoted 10 lines
scripts/checkkconfigsymbols.py +2
-2-def execute(cmd): +def execute(cmd, stderr=subprocess.STDOUT): """Execute %cmd and return stdout. Exit in case of error.""" try: - stdout = subprocess.check_output(cmd, stderr=subprocess.STDOUT, shell=False) + stdout = subprocess.check_output(cmd, stderr=stderr, shell=False) stdout = stdout.decode(errors='replace') except subprocess.CalledProcessError as fail: exit(fail)
This patch is unrelated to your patch 1/2, so they should be sent separately.
Sets of patches are intended for changes that depend on each other, or at least are closely related.
- Julian Braha
Julian Braha
reviewerHi Erkan,
I see that this is your first patch submission to the kernel. Welcome!
First thing, if you run the ./scripts/get_maintainer.pl check on this patch, it should list a few more people to CC.
Quoted 20 lines · Erkan Erdem
conf_string()unconditionally removes the last character returned byfgets(), assuming that it is a newline. When redirected input ends without a newline, the last character is part of the answer instead. For example, feeding 42 to an integer prompt stores 4, and feeding0xffto a hexadecimal prompt stores0xf. Both commands succeed despite silently changing the supplied value.Strip the newline with
strcspn()so that a complete answer at EOF is preserved. Keep the existing handling of newline-terminated and empty answers unchanged.---
An AI coding assistant found the issue, prepared the fix and changelog, and ran the verification below after a request to find reproducible functional bugs in Linux development tools.
Quoted 9 lines
Validation:
- Built conf with Clang on macOS and GCC in anx86_64Linux container, using -Wall -Wmissing-prototypes -Wstrict-prototypes -Werror.
- Ran 60 before/after executions on each platform, covering oldaskconfig and oldconfig with string, int and hex answers. Unterminated answers retain their last character. Newline, empty-answer, empty-input and CRLF controls keep the same results.
- The 21 existingKconfigtests pass on macOS.
- This tests the host configuration tool; a complete kernel was not built.
Since you already have some tests, it would be good to add a basic test for this in scripts/kconfig/tests.
Kconfig doesn't have as many tests as it should, and it's been complained about by some core maintainers in the past [1].
Quoted 18 lines
scripts/kconfig/conf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)scripts/kconfig/conf.c +1
-1@@ -343,7 +343,7 @@ static int conf_string(struct menu *menu) }/* fall through */ default : - line[strlen (line)-1 ] =0 ; + line[strcspn (line,"\n" )] =0 ; def = line; }if (def &&sym_set_string_value (sym, def))
Quoted 1 line
base-commit:
4d7d9486c04d917265f64c55bd23b2cc4fe7749c
Otherwise this change looks good to me (and is more readable).
- Julian Braha