Re: [PATCH v5 22/31] KVM: s390: Use arm64 code

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] The generated `#undef __INCL_GEN_ARM_FILE` is placed inside the `#ifndef __INCL_GEN_ARM_FILE` block, making it dead code.
- [Low] The awk script fails to detect unbalanced/duplicate end markers.
--

--- Patch [22]: [PATCH v5 22/31] KVM: s390: Use arm64 code ---
commit ae7db48a91d4b61d3d1ccc6f0b0c6c7d8f856a81
Author: Steffen Eiden <[email protected]>

KVM: s390: Use arm64 code

This commit adds infrastructure to extract marked sections of ARM64 KVM code
into generated .inc files for sharing with s390. It introduces an awk script
and Makefile rules to detect start and end markers and extract the code at
build time.

> diff --git a/arch/s390/kvm/arm64/copy-arm64c.awk b/arch/s390/kvm/arm64/copy-arm64c.awk
> new file mode 100644
> index 0000000000000..6c4fd1f080982
> --- /dev/null
> +++ b/arch/s390/kvm/arm64/copy-arm64c.awk

[ ... ]

> +	print " * This file was automatically generated from " filename
> +	print " * Do not modify this file directly."
> +	print " */"
> +	print ""
> +	print "#ifndef __INCL_GEN_ARM_FILE"
> +	print "#error included .inc file w/o proper guard definition"
> +	print "#undef __INCL_GEN_ARM_FILE"

[Severity: Low]
This isn't a bug, but is this generated undef reachable?

Since it is emitted inside the #ifndef __INCL_GEN_ARM_FILE block, if the
macro is not defined, the compiler will halt at the #error. If the macro
is defined, the preprocessor will skip this entire block.

Should this undef be removed, since the commit message states that the
consumer must undef it themselves?

> +	print "#endif /* __INCL_GEN_ARM_FILE */"
> +	print ""
> +
> +	file_header_done = 1
> +}
> +
> +$0 ~ start_pattern {
> +	if (copying) { exit 1 }
> +	copying = found_marker = 1
> +	next
> +}
> +
> +$0 ~ end_pattern {
> +	copying = 0

[Severity: Low]
Does this correctly handle an unbalanced end marker?

While the start_pattern rule catches nested start markers by checking
if copying is already set, this end_pattern rule just resets the state.

If a source file has an extra end marker without a preceding start marker,
wouldn't this silently process it instead of failing as the Makefile's
"unbalanced" error message suggests?

Could we add a check like if (!copying) { exit 1 } here to properly
catch extraneous end markers?

> +	next
> +}
> +
> +copying {

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=22