Re: Makefile.lib: cmd_dtc: warning: missing whitespace after the macro name

[Sascha Hauer <s.hauer@pengutronix.de>]

On Wed, Sep 07, 2022 at 11:21:18AM +0300, Antony Pavlov wrote:
> Hi Everyone!
> 
> During MIPS ath79_defconfig build I have a 'missing whitespace after the macro name' warning:
> 
>   AS [P]  arch/mips/dts/ar9331_tl_mr3020.dtb.pbl.o
>   DTC     arch/mips/dts/ar9344-tl-wdr4300-v1.7.dtb
> <stdin>:1:9: warning: missing whitespace after the macro name
>   XZKERN  arch/mips/dts/ar9344-tl-wdr4300-v1.7.dtb.z
> 
> E.g. see https://gitlab.com/frantony/barebox/-/jobs/2969826747#L47
> 
> The reason is the scripts/Makefile.lib generates a C macro with the point symbol in the macro name because the arch/mips/dts/ar9344-tl-wdr4300-v1.7.dts file name contains the point symbol before '.dts', as a result we have:
> 
>   #define ar9344_tl_wdr4300_v1.7_dts 1
> 
> e.g.
> 
>   barebox$ grep -RHn -o "define ar9344_tl_wdr4300_v1.* 1" . 2>/dev/null
>   ./arch/mips/dts/.ar9344-tl-wdr4300-v1.7.dtb.cmd:1:define ar9344_tl_wdr4300_v1.7_dts 1
> 
> cmd_dtc in scripts/Makefile.lib substitutes the '-' symbols with the '_' symbols but do nothing with other unwanted C preprocessor macro name symbols.
> 
> It looks like the linux kernel has no problems with extra point symbols in dts file names, there are several files with extra dot in dts:
> 
>   barebox$ find dts/ -iname '*.*.dts' | wc -l
>   33
> 
> So we have to fix Makefile.lib.
> 
> This simple patch fixes the warning problem:
> 
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index 16308497b84..2f79656c1e9 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -363,7 +363,7 @@ $(obj)/%.dtb.z: $(obj)/%.dtb FORCE
>  dts-frags = $(subst $(quote),,$(CONFIG_EXTERNAL_DTS_FRAGMENTS))
>  quiet_cmd_dtc = DTC     $@
>  # For compatibility between make 4.2 and 4.3
> -cmd_dtc = /usr/bin/env echo -e '$(pound)define $(subst -,_,$(*F))_dts 1\n'$(foreach f,$< $(dts-frags),'$(pound)include "$(f)"\n') | \
> +cmd_dtc = /usr/bin/env echo -e '$(pound)define $(subst -,_,$(subst .,_,$(*F)))_dts 1\n'$(foreach f,$< $(dts-frags),'$(pound)include "$(f)"\n') | \
>  	$(CPP) $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) - ; \
>  	$(objtree)/scripts/dtc/dtc -O dtb -o $@ -b 0 \
>  		-i $(srctree)/arch/$(SRCARCH)/dts $(DTC_FLAGS) \
> 
> I suppose that this simple patch may lead to some undesirable side effects.

One side effect is that this gets even less readable.

Another one would be that two dts filenames which only differ in the
usage of '.' and '_' would result in the same define, but I think that
case is negligible as this define is unused in barebox itself. It could
be used by external dts fragments passed in via CONFIG_EXTERNAL_DTS_FRAGMENTS.

Other than that, what side effects are you afraid of?

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |