Re: [PATCH 2/2] checkkconfigsymbols: resolve revisions before resetting the tree

[Julian Braha]

On 9/5/26 13:32, Erkan Erdem wrote:
> 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.
> 
> Fixes: b1a3f243485f ("checkkconfigsymbols.py: make it Git aware")
> Link: https://lore.kernel.org/20210901145212.478066-1-arielmarcovitch@xxxxxxxxx/
> Assisted-by: LLM
> Signed-off-by: Erkan Erdem <hexvalid@xxxxxxxxx>
> ---
> 
> 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(-)
> 
> diff --git a/scripts/checkkconfigsymbols.py b/scripts/checkkconfigsymbols.py
> index 36c920e7..e0a1a631 100755
> --- a/scripts/checkkconfigsymbols.py
> +++ b/scripts/checkkconfigsymbols.py
> @@ -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)

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