[bpf-next] kbuild: replace BASH-specific ${@:2} with shift and ${@}
diff mbox series

Message ID 20190905175938.599455-1-andriin@fb.com
State Accepted
Delegated to: BPF Maintainers
Headers show
Series
  • [bpf-next] kbuild: replace BASH-specific ${@:2} with shift and ${@}
Related show

Commit Message

Andrii Nakryiko Sept. 5, 2019, 5:59 p.m. UTC
${@:2} is BASH-specific extension, which makes link-vmlinux.sh rely on
BASH. Use shift and ${@} instead to fix this issue.

Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Fixes: 341dfcf8d78e ("btf: expose BTF info through sysfs")
Cc: Stephen Rothwell <sfr@canb.auug.org.au>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 scripts/link-vmlinux.sh | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

Comments

Yonghong Song Sept. 5, 2019, 11:59 p.m. UTC | #1
On 9/5/19 10:59 AM, Andrii Nakryiko wrote:
> ${@:2} is BASH-specific extension, which makes link-vmlinux.sh rely on
> BASH. Use shift and ${@} instead to fix this issue.
> 
> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> Fixes: 341dfcf8d78e ("btf: expose BTF info through sysfs")
> Cc: Stephen Rothwell <sfr@canb.auug.org.au>
> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>

Tested with bash/sh/csh, all works.
Acked-by: Yonghong Song <yhs@fb.com>

> ---
>   scripts/link-vmlinux.sh | 16 +++++++++++-----
>   1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> index 0d8f41db8cd6..8c59970a09dc 100755
> --- a/scripts/link-vmlinux.sh
> +++ b/scripts/link-vmlinux.sh
> @@ -57,12 +57,16 @@ modpost_link()
>   
>   # Link of vmlinux
>   # ${1} - output file
> -# ${@:2} - optional extra .o files
> +# ${2}, ${3}, ... - optional extra .o files
>   vmlinux_link()
>   {
>   	local lds="${objtree}/${KBUILD_LDS}"
> +	local output=${1}
>   	local objects
>   
> +	# skip output file argument
> +	shift
> +
>   	if [ "${SRCARCH}" != "um" ]; then
>   		objects="--whole-archive			\
>   			${KBUILD_VMLINUX_OBJS}			\
> @@ -70,9 +74,10 @@ vmlinux_link()
>   			--start-group				\
>   			${KBUILD_VMLINUX_LIBS}			\
>   			--end-group				\
> -			${@:2}"
> +			${@}"
>   
> -		${LD} ${KBUILD_LDFLAGS} ${LDFLAGS_vmlinux} -o ${1}	\
> +		${LD} ${KBUILD_LDFLAGS} ${LDFLAGS_vmlinux}	\
> +			-o ${output}				\
>   			-T ${lds} ${objects}
>   	else
>   		objects="-Wl,--whole-archive			\
> @@ -81,9 +86,10 @@ vmlinux_link()
>   			-Wl,--start-group			\
>   			${KBUILD_VMLINUX_LIBS}			\
>   			-Wl,--end-group				\
> -			${@:2}"
> +			${@}"
>   
> -		${CC} ${CFLAGS_vmlinux} -o ${1}			\
> +		${CC} ${CFLAGS_vmlinux}				\
> +			-o ${output}				\
>   			-Wl,-T,${lds}				\
>   			${objects}				\
>   			-lutil -lrt -lpthread
>
Masahiro Yamada Sept. 6, 2019, 3:53 a.m. UTC | #2
> -----Original Message-----
> From: Andrii Nakryiko <andriin@fb.com>
> Sent: Friday, September 06, 2019 3:00 AM
> To: bpf@vger.kernel.org; netdev@vger.kernel.org; ast@fb.com;
> daniel@iogearbox.net
> Cc: andrii.nakryiko@gmail.com; kernel-team@fb.com; Andrii Nakryiko
> <andriin@fb.com>; Stephen Rothwell <sfr@canb.auug.org.au>; Yamada,
> Masahiro/山田 真弘 <yamada.masahiro@socionext.com>
> Subject: [PATCH bpf-next] kbuild: replace BASH-specific ${@:2} with shift
> and ${@}
> 
> ${@:2} is BASH-specific extension, which makes link-vmlinux.sh rely on
> BASH. Use shift and ${@} instead to fix this issue.
> 
> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> Fixes: 341dfcf8d78e ("btf: expose BTF info through sysfs")
> Cc: Stephen Rothwell <sfr@canb.auug.org.au>
> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>

Reviewed-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Andrii Nakryiko Sept. 6, 2019, 9:11 a.m. UTC | #3
On 9/6/19 12:59 AM, Yonghong Song wrote:
> 
> 
> On 9/5/19 10:59 AM, Andrii Nakryiko wrote:
>> ${@:2} is BASH-specific extension, which makes link-vmlinux.sh rely on
>> BASH. Use shift and ${@} instead to fix this issue.
>>
>> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
>> Fixes: 341dfcf8d78e ("btf: expose BTF info through sysfs")
>> Cc: Stephen Rothwell <sfr@canb.auug.org.au>
>> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
>> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> 
> Tested with bash/sh/csh, all works.

Thanks for testing, Yonghong! In my system sh is an alias to bash, so it 
still behaved like bash and didn't fail even with existing code. I 
didn't have an opportunity to install csh at that time and try it, so 
thanks a lot for confirming. I basically relied on documentation to 
verify shift and $@ is not BASH'ism.

> Acked-by: Yonghong Song <yhs@fb.com>
> 
>> ---
>>    scripts/link-vmlinux.sh | 16 +++++++++++-----
>>    1 file changed, 11 insertions(+), 5 deletions(-)
>>
>> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
>> index 0d8f41db8cd6..8c59970a09dc 100755
>> --- a/scripts/link-vmlinux.sh
>> +++ b/scripts/link-vmlinux.sh
>> @@ -57,12 +57,16 @@ modpost_link()
>>    
>>    # Link of vmlinux
>>    # ${1} - output file
>> -# ${@:2} - optional extra .o files
>> +# ${2}, ${3}, ... - optional extra .o files
>>    vmlinux_link()
>>    {
>>    	local lds="${objtree}/${KBUILD_LDS}"
>> +	local output=${1}
>>    	local objects
>>    
>> +	# skip output file argument
>> +	shift
>> +
>>    	if [ "${SRCARCH}" != "um" ]; then
>>    		objects="--whole-archive			\
>>    			${KBUILD_VMLINUX_OBJS}			\
>> @@ -70,9 +74,10 @@ vmlinux_link()
>>    			--start-group				\
>>    			${KBUILD_VMLINUX_LIBS}			\
>>    			--end-group				\
>> -			${@:2}"
>> +			${@}"
>>    
>> -		${LD} ${KBUILD_LDFLAGS} ${LDFLAGS_vmlinux} -o ${1}	\
>> +		${LD} ${KBUILD_LDFLAGS} ${LDFLAGS_vmlinux}	\
>> +			-o ${output}				\
>>    			-T ${lds} ${objects}
>>    	else
>>    		objects="-Wl,--whole-archive			\
>> @@ -81,9 +86,10 @@ vmlinux_link()
>>    			-Wl,--start-group			\
>>    			${KBUILD_VMLINUX_LIBS}			\
>>    			-Wl,--end-group				\
>> -			${@:2}"
>> +			${@}"
>>    
>> -		${CC} ${CFLAGS_vmlinux} -o ${1}			\
>> +		${CC} ${CFLAGS_vmlinux}				\
>> +			-o ${output}				\
>>    			-Wl,-T,${lds}				\
>>    			${objects}				\
>>    			-lutil -lrt -lpthread
>>
Alexei Starovoitov Sept. 6, 2019, 5:03 p.m. UTC | #4
On Fri, Sep 6, 2019 at 3:34 AM <yamada.masahiro@socionext.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Andrii Nakryiko <andriin@fb.com>
> > Sent: Friday, September 06, 2019 3:00 AM
> > To: bpf@vger.kernel.org; netdev@vger.kernel.org; ast@fb.com;
> > daniel@iogearbox.net
> > Cc: andrii.nakryiko@gmail.com; kernel-team@fb.com; Andrii Nakryiko
> > <andriin@fb.com>; Stephen Rothwell <sfr@canb.auug.org.au>; Yamada,
> > Masahiro/山田 真弘 <yamada.masahiro@socionext.com>
> > Subject: [PATCH bpf-next] kbuild: replace BASH-specific ${@:2} with shift
> > and ${@}
> >
> > ${@:2} is BASH-specific extension, which makes link-vmlinux.sh rely on
> > BASH. Use shift and ${@} instead to fix this issue.
> >
> > Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> > Fixes: 341dfcf8d78e ("btf: expose BTF info through sysfs")
> > Cc: Stephen Rothwell <sfr@canb.auug.org.au>
> > Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
>
> Reviewed-by: Masahiro Yamada <yamada.masahiro@socionext.com>

Applied. Thanks

Patch
diff mbox series

diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index 0d8f41db8cd6..8c59970a09dc 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -57,12 +57,16 @@  modpost_link()
 
 # Link of vmlinux
 # ${1} - output file
-# ${@:2} - optional extra .o files
+# ${2}, ${3}, ... - optional extra .o files
 vmlinux_link()
 {
 	local lds="${objtree}/${KBUILD_LDS}"
+	local output=${1}
 	local objects
 
+	# skip output file argument
+	shift
+
 	if [ "${SRCARCH}" != "um" ]; then
 		objects="--whole-archive			\
 			${KBUILD_VMLINUX_OBJS}			\
@@ -70,9 +74,10 @@  vmlinux_link()
 			--start-group				\
 			${KBUILD_VMLINUX_LIBS}			\
 			--end-group				\
-			${@:2}"
+			${@}"
 
-		${LD} ${KBUILD_LDFLAGS} ${LDFLAGS_vmlinux} -o ${1}	\
+		${LD} ${KBUILD_LDFLAGS} ${LDFLAGS_vmlinux}	\
+			-o ${output}				\
 			-T ${lds} ${objects}
 	else
 		objects="-Wl,--whole-archive			\
@@ -81,9 +86,10 @@  vmlinux_link()
 			-Wl,--start-group			\
 			${KBUILD_VMLINUX_LIBS}			\
 			-Wl,--end-group				\
-			${@:2}"
+			${@}"
 
-		${CC} ${CFLAGS_vmlinux} -o ${1}			\
+		${CC} ${CFLAGS_vmlinux}				\
+			-o ${output}				\
 			-Wl,-T,${lds}				\
 			${objects}				\
 			-lutil -lrt -lpthread