Re: [PATCH v5 3/4] dt-bindings: wire style checker into dt_binding_check

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `cmd_chk_style` recipe contains implementation flaws: failure to connect to the GNU Make jobserver and unhandled build failure on empty file lists.
--

commit a0bf52b4283b270bea18b1e64b7f4c19cfa75a3e
Author: Daniel Golle <daniel@makrotopia.org>

dt-bindings: wire style checker into dt_binding_check

This commit wires the new dt-check-style tool into the dt_binding_check
target. It uses scripts/jobserver-exec to constrain the number of workers
based on the GNU make jobserver slots, defaulting to relaxed mode.

> diff --git a/Documentation/devicetree/bindings/Makefile b/Documentation/devicetree/bindings/Makefile
> --- a/Documentation/devicetree/bindings/Makefile
> +++ b/Documentation/devicetree/bindings/Makefile
> @@ -46,6 +46,18 @@ quiet_cmd_chk_bindings = CHKDT   $(src)
>  			  xargs -n200 -P$$(nproc) $(DT_DOC_CHECKER) -u $(src)) \
>  			  && touch $@ || true
>  
> +DT_CHK_STYLE = $(srctree)/scripts/dtc/dt-check-style
> +
> +# Feed the file list to the checker via @argfile in a single Python
> +# process so the ruamel.yaml import is paid once. scripts/jobserver-exec
> +# claims slots from the GNU make jobserver and exposes the count via
> +# $PARALLELISM, which dt-check-style picks up to size its worker pool.
> +quiet_cmd_chk_style = STYLE   $(src)
> +      cmd_chk_style = f=$$(mktemp) && $(find_cmd) > $$f && \
> +		      $(PYTHON3) $(srctree)/scripts/jobserver-exec \
> +		      $(PYTHON3) $(DT_CHK_STYLE) @$$f \

[Severity: Medium]
What happens if the file list is empty (e.g., when filtering with
DT_SCHEMA_FILES)?

If $(find_cmd) matches no files, the temporary file $$f will be empty.
Passing @$$f to dt-check-style when it is empty could result in zero
arguments, triggering an argparse error that abruptly fails the build
instead of gracefully skipping the check.

> +		      && touch $@ || true; rm -f $$f
> +

[ ... ]

> @@ -62,13 +74,16 @@ override DTC_FLAGS := \
>  $(obj)/processed-schema.json: $(DT_DOCS) check_dtschema_version FORCE
>  	$(call if_changed,mk_schema)
>  
> -targets += .dt-binding.checked .yamllint.checked
> +targets += .dt-binding.checked .yamllint.checked .dt-style.checked
>  $(obj)/.yamllint.checked: $(DT_DOCS) $(src)/.yamllint FORCE
>  	$(if $(DT_SCHEMA_LINT),$(call if_changed,yamllint),)
>  
>  $(obj)/.dt-binding.checked: $(DT_DOCS) FORCE
>  	$(call if_changed,chk_bindings)
>  
> +$(obj)/.dt-style.checked: $(DT_DOCS) FORCE
> +	$(call if_changed,chk_style)

[Severity: Medium]
Does this recipe successfully connect to the GNU make jobserver?

The recipe uses $(call if_changed,chk_style) without the '+' prefix required
for recursive make. 

Without this prefix, GNU make (prior to v4.4) closes the jobserver file
descriptors before executing the shell. This causes jobserver-exec to fail
to reopen them and silently default to os.cpu_count(), ignoring the user's
-j constraint and potentially overloading the system.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1779908995.git.daniel@makrotopia.org?part=3