Re: [PATCH v2 2/2] objtool: Move sync check to a script

[Ingo Molnar <mingo@kernel.org>]

* Josh Poimboeuf <jpoimboe@redhat.com> wrote:

> Replace the nasty diff checks in the objtool Makefile with a clean bash
> script, and make the warnings more specific.
> 
> Heavily inspired by tools/perf/check-headers.sh.
> 
> Suggested-by: Ingo Molnar <mingo@kernel.org>
> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
> ---
>  tools/objtool/Makefile      | 16 +---------------
>  tools/objtool/sync-check.sh | 29 +++++++++++++++++++++++++++++
>  2 files changed, 30 insertions(+), 15 deletions(-)
>  create mode 100755 tools/objtool/sync-check.sh
> 
> diff --git a/tools/objtool/Makefile b/tools/objtool/Makefile
> index c6a19d946ec1..6aaed251b4ed 100644
> --- a/tools/objtool/Makefile
> +++ b/tools/objtool/Makefile
> @@ -43,22 +43,8 @@ include $(srctree)/tools/build/Makefile.include
>  $(OBJTOOL_IN): fixdep FORCE
>  	@$(MAKE) $(build)=objtool
>  
> -# Busybox's diff doesn't have -I, avoid warning in that case
> -#
>  $(OBJTOOL): $(LIBSUBCMD) $(OBJTOOL_IN)
> -	@(diff -I 2>&1 | grep -q 'option requires an argument' && \
> -	test -d ../../kernel -a -d ../../tools -a -d ../objtool && (( \
> -	diff arch/x86/lib/insn.c ../../arch/x86/lib/insn.c >/dev/null && \
> -	diff arch/x86/lib/inat.c ../../arch/x86/lib/inat.c >/dev/null && \
> -	diff arch/x86/lib/x86-opcode-map.txt ../../arch/x86/lib/x86-opcode-map.txt >/dev/null && \
> -	diff arch/x86/tools/gen-insn-attr-x86.awk ../../arch/x86/tools/gen-insn-attr-x86.awk >/dev/null && \
> -	diff arch/x86/include/asm/insn.h ../../arch/x86/include/asm/insn.h >/dev/null && \
> -	diff arch/x86/include/asm/inat.h ../../arch/x86/include/asm/inat.h >/dev/null && \
> -	diff arch/x86/include/asm/inat_types.h ../../arch/x86/include/asm/inat_types.h >/dev/null) \
> -	|| echo "warning: objtool: x86 instruction decoder differs from kernel" >&2 )) || true
> -	@(test -d ../../kernel -a -d ../../tools -a -d ../objtool && (( \
> -	diff ../../arch/x86/include/asm/orc_types.h arch/x86/include/asm/orc_types.h >/dev/null) \
> -	|| echo "warning: objtool: orc_types.h differs from kernel" >&2 )) || true
> +	@./sync-check.sh
>  	$(QUIET_LINK)$(CC) $(OBJTOOL_IN) $(LDFLAGS) -o $@
>  
>  
> diff --git a/tools/objtool/sync-check.sh b/tools/objtool/sync-check.sh
> new file mode 100755
> index 000000000000..f2e5f8b0460c
> --- /dev/null
> +++ b/tools/objtool/sync-check.sh
> @@ -0,0 +1,29 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0
> +
> +FILES='
> +arch/x86/lib/insn.c
> +arch/x86/lib/inat.c
> +arch/x86/lib/x86-opcode-map.txt
> +arch/x86/tools/gen-insn-attr-x86.awk
> +arch/x86/include/asm/insn.h
> +arch/x86/include/asm/inat.h
> +arch/x86/include/asm/inat_types.h
> +arch/x86/include/asm/orc_types.h
> +'
> +
> +check()
> +{
> +	local file=$1
> +
> +	diff $file ../../$file &> /dev/null ||
> +		echo "Warning: synced file at 'tools/objtool/$file' differs from latest kernel version at '$file'"
> +}
> +
> +if [ ! -d ../../kernel ] || [ ! -d ../../tools ] || [ ! -d ../objtool ]; then
> +	exit 0
> +fi
> +
> +for i in $FILES; do
> +  check $i
> +done

Hm, this doesn't actually warn - it outputs the diff:

 triton:~/tip/tools/objtool> ./sync-check.sh 
 triton:~/tip/tools/objtool> 99a100,109
 > /* Identifiers for segment registers */
 > #define INAT_SEG_REG_IGNORE   0
 > #define INAT_SEG_REG_DEFAULT  1
 > #define INAT_SEG_REG_CS               2
 > #define INAT_SEG_REG_SS               3
 > #define INAT_SEG_REG_DS               4
 > #define INAT_SEG_REG_ES               5
 > #define INAT_SEG_REG_FS               6
 > #define INAT_SEG_REG_GS               7
 > 

I fixed it to do:

        diff $file ../../$file > /dev/null ||

(note the removal of '&')

Then it outputs the right thing:

  triton:~/tip/tools/objtool> ./sync-check.sh 
  Warning: synced file at 'tools/objtool/arch/x86/include/asm/inat.h' differs from latest kernel version at 'arch/x86/include/asm/inat.h'

Thanks,

	Ingo