Re: [RFC PATCH 2/5] kbuild: dedumplicated the generating code

[Jan Beulich]

>>> On 28.03.11 at 04:58, Lai Jiangshan <laijs@xxxxxxxxxxxxxx> wrote:

> 
> cmd_bounds and cmd_offsets are almost the same, dedumplicated them.
> The definition of sed-y is also moved up the top of the file.
> 
> Signed-off-by: Lai Jiangshan <laijs@xxxxxxxxxxxxxx>
> ---
>  Kbuild |   60 +++++++++++++++++++++---------------------------------------
>  1 files changed, 21 insertions(+), 39 deletions(-)
> 
> diff --git a/Kbuild b/Kbuild
> index 2114113..5c2ee83 100644
> --- a/Kbuild
> +++ b/Kbuild
> @@ -5,19 +5,19 @@
>  # 2) Generate asm-offsets.h (may need bounds.h)
>  # 3) Check for missing system calls
>  
> -#####
> -# 1) Generate bounds.h
> -
> -bounds-file := include/generated/bounds.h
> -
> -always  := $(bounds-file)
> -targets := $(bounds-file) kernel/bounds.s
> +# Default sed regexp - multiline due to syntax constraints
> +define sed-y
> +	"/^->/{s:->#\(.*\):/* \1 */:; \
> +	s:^->\([^ ]*\) [\$$#]*\([-0-9]*\) \(.*\):#define \1 \2 /* \3 */:; \
> +	s:^->\([^ ]*\) [\$$#]*\([^ ]*\) \(.*\):#define \1 \2 /* \3 */:; \

Having to touch (move) this code would imo be a very good
opportunity to simplify it again: The replacement string is the
same for those two lines, and the second match expression is
a super set of the first one. That is, the first one can be
dropped, and hence the whole thing changed back to its
pre-3234282f33b29d349bcada40204fc7c8fda7fe72 form.

Jan

> +	s:->::; p;}"
> +endef
>  
> -quiet_cmd_bounds = GEN     $@
> -define cmd_bounds
> +quiet_cmd_kbuild_gen = GEN     $@
> +define cmd_kbuild_gen
>  	(set -e; \
> -	 echo "#ifndef __LINUX_BOUNDS_H__"; \
> -	 echo "#define __LINUX_BOUNDS_H__"; \
> +	 echo "#ifndef __$$(echo $@ | tr -- '-./a-z' '___A-Z')__"; \
> +	 echo "#define __$$(echo $@ | tr -- '-./a-z' '___A-Z')__"; \
>  	 echo "/*"; \
>  	 echo " * DO NOT MODIFY."; \
>  	 echo " *"; \
> @@ -30,6 +30,14 @@ define cmd_bounds
>  	 echo "#endif" ) > $@
>  endef
>  
> +#####
> +# 1) Generate bounds.h
> +
> +bounds-file := include/generated/bounds.h
> +
> +always  := $(bounds-file)
> +targets := $(bounds-file) kernel/bounds.s
> +
>  # We use internal kbuild rules to avoid the "is up to date" message from 
> make
>  kernel/bounds.s: kernel/bounds.c FORCE
>  	$(Q)mkdir -p $(dir $@)
> @@ -37,7 +45,7 @@ kernel/bounds.s: kernel/bounds.c FORCE
>  
>  $(obj)/$(bounds-file): kernel/bounds.s Kbuild
>  	$(Q)mkdir -p $(dir $@)
> -	$(call cmd,bounds)
> +	$(call cmd,kbuild_gen)
>  
>  #####
>  # 2) Generate asm-offsets.h
> @@ -49,32 +57,6 @@ always  += $(offsets-file)
>  targets += $(offsets-file)
>  targets += arch/$(SRCARCH)/kernel/asm-offsets.s
>  
> -
> -# Default sed regexp - multiline due to syntax constraints
> -define sed-y
> -	"/^->/{s:->#\(.*\):/* \1 */:; \
> -	s:^->\([^ ]*\) [\$$#]*\([-0-9]*\) \(.*\):#define \1 \2 /* \3 */:; \
> -	s:^->\([^ ]*\) [\$$#]*\([^ ]*\) \(.*\):#define \1 \2 /* \3 */:; \
> -	s:->::; p;}"
> -endef
> -
> -quiet_cmd_offsets = GEN     $@
> -define cmd_offsets
> -	(set -e; \
> -	 echo "#ifndef __ASM_OFFSETS_H__"; \
> -	 echo "#define __ASM_OFFSETS_H__"; \
> -	 echo "/*"; \
> -	 echo " * DO NOT MODIFY."; \
> -	 echo " *"; \
> -	 echo " * This file was generated by Kbuild"; \
> -	 echo " *"; \
> -	 echo " */"; \
> -	 echo ""; \
> -	 sed -ne $(sed-y) $<; \
> -	 echo ""; \
> -	 echo "#endif" ) > $@
> -endef
> -
>  # We use internal kbuild rules to avoid the "is up to date" message from 
> make
>  arch/$(SRCARCH)/kernel/asm-offsets.s: arch/$(SRCARCH)/kernel/asm-offsets.c 
> \
>                                        $(obj)/$(bounds-file) FORCE
> @@ -82,7 +64,7 @@ arch/$(SRCARCH)/kernel/asm-offsets.s: 
> arch/$(SRCARCH)/kernel/asm-offsets.c \
>  	$(call if_changed_dep,cc_s_c)
>  
>  $(obj)/$(offsets-file): arch/$(SRCARCH)/kernel/asm-offsets.s Kbuild
> -	$(call cmd,offsets)
> +	$(call cmd,kbuild_gen)
>  
>  #####
>  # 3) Check for missing system calls
> -- 
> 1.7.4



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/