| Message ID | 20220210190642.1477814-2-usama.anjum@collabora.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | selftests: sgx: Fix build of test_sgx | expand |
On 2/10/22 12:06 PM, Muhammad Usama Anjum wrote: > CC can have multiple sub-strings like "ccache gcc". Erorr pops up if > it is treated as single string and double quote are used around it. > This can be fixed by removing the quotes and not treating CC a single > string. > > Fixes: e9886ace222e ("selftests, x86: Rework x86 target architecture detection") > Reported-by: "kernelci.org bot" <bot@kernelci.org> > Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com> > --- > tools/testing/selftests/x86/check_cc.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tools/testing/selftests/x86/check_cc.sh b/tools/testing/selftests/x86/check_cc.sh > index 3e2089c8cf549..aff2c15018b53 100755 > --- a/tools/testing/selftests/x86/check_cc.sh > +++ b/tools/testing/selftests/x86/check_cc.sh > @@ -7,7 +7,7 @@ CC="$1" > TESTPROG="$2" > shift 2 > > -if "$CC" -o /dev/null "$TESTPROG" -O0 "$@" 2>/dev/null; then > +if $CC -o /dev/null "$TESTPROG" -O0 "$@" 2>/dev/null; then > echo 1 > else > echo 0 > The intent is testing if $CC is set. Does this change work when $CC is not set? thanks, -- Shuah
On 2/11/22 1:51 AM, Shuah Khan wrote: > On 2/10/22 12:06 PM, Muhammad Usama Anjum wrote: >> CC can have multiple sub-strings like "ccache gcc". Erorr pops up if >> it is treated as single string and double quote are used around it. >> This can be fixed by removing the quotes and not treating CC a single >> string. >> >> Fixes: e9886ace222e ("selftests, x86: Rework x86 target architecture >> detection") >> Reported-by: "kernelci.org bot" <bot@kernelci.org> >> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com> >> --- >> tools/testing/selftests/x86/check_cc.sh | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/tools/testing/selftests/x86/check_cc.sh >> b/tools/testing/selftests/x86/check_cc.sh >> index 3e2089c8cf549..aff2c15018b53 100755 >> --- a/tools/testing/selftests/x86/check_cc.sh >> +++ b/tools/testing/selftests/x86/check_cc.sh >> @@ -7,7 +7,7 @@ CC="$1" >> TESTPROG="$2" >> shift 2 >> -if "$CC" -o /dev/null "$TESTPROG" -O0 "$@" 2>/dev/null; then >> +if $CC -o /dev/null "$TESTPROG" -O0 "$@" 2>/dev/null; then >> echo 1 >> else >> echo 0 >> > > The intent is testing if $CC is set. Does this change work when > $CC is not set? > Yeah, it works. I've added a debug variable inside sgx/Makefile and it is detecting empty argument correctly as well. --- a/tools/testing/selftests/sgx/Makefile +++ b/tools/testing/selftests/sgx/Makefile @@ -6,7 +6,7 @@ include ../lib.mk CAN_BUILD_X86_64 := $(shell ../x86/check_cc.sh "$(CC)" \ ../x86/trivial_64bit_program.c) - +$(info $$CAN_BUILD_X86_64 is [${CAN_BUILD_X86_64}]) Wrong examples: ➜ sgx (next-20220210_) ✗ make CC="" $CAN_BUILD_X86_64 is [0] ➜ sgx (next-20220210_) ✗ make CC="cache gcc" $CAN_BUILD_X86_64 is [0] Correct examples: ➜ sgx (next-20220210_) ✗ make CC=gcc $CAN_BUILD_X86_64 is [1] ➜ sgx (next-20220210_) ✗ make $CAN_BUILD_X86_64 is [1] ➜ sgx (next-20220210_) ✗ make CC="ccache gcc" $CAN_BUILD_X86_64 is [1] ➜ sgx (next-20220210_) ✗ make CC="gcc" $CAN_BUILD_X86_64 is [1] ➜ sgx (next-20220210_) ✗ make CC="clang" $CAN_BUILD_X86_64 is [1] ➜ sgx (next-20220210_) ✗ make CC="ccache clang" $CAN_BUILD_X86_64 is [1]
On 2/10/22 3:41 PM, Muhammad Usama Anjum wrote: > On 2/11/22 1:51 AM, Shuah Khan wrote: >> On 2/10/22 12:06 PM, Muhammad Usama Anjum wrote: >>> CC can have multiple sub-strings like "ccache gcc". Erorr pops up if >>> it is treated as single string and double quote are used around it. >>> This can be fixed by removing the quotes and not treating CC a single >>> string. >>> >>> Fixes: e9886ace222e ("selftests, x86: Rework x86 target architecture >>> detection") >>> Reported-by: "kernelci.org bot" <bot@kernelci.org> >>> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com> >>> --- >>> tools/testing/selftests/x86/check_cc.sh | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/tools/testing/selftests/x86/check_cc.sh >>> b/tools/testing/selftests/x86/check_cc.sh >>> index 3e2089c8cf549..aff2c15018b53 100755 >>> --- a/tools/testing/selftests/x86/check_cc.sh >>> +++ b/tools/testing/selftests/x86/check_cc.sh >>> @@ -7,7 +7,7 @@ CC="$1" >>> TESTPROG="$2" >>> shift 2 >>> -if "$CC" -o /dev/null "$TESTPROG" -O0 "$@" 2>/dev/null; then >>> +if $CC -o /dev/null "$TESTPROG" -O0 "$@" 2>/dev/null; then >>> echo 1 >>> else >>> echo 0 >>> >> >> The intent is testing if $CC is set. Does this change work when >> $CC is not set? >> > Yeah, it works. I've added a debug variable inside sgx/Makefile and it > is detecting empty argument correctly as well. > Sounds good. thanks, -- Shuah
On 2/11/22 9:47 AM, David Laight wrote: > From: Shuah Khan >> Sent: 10 February 2022 20:52 >> >> On 2/10/22 12:06 PM, Muhammad Usama Anjum wrote: >>> CC can have multiple sub-strings like "ccache gcc". Erorr pops up if >>> it is treated as single string and double quote are used around it. >>> This can be fixed by removing the quotes and not treating CC a single >>> string. >>> >>> Fixes: e9886ace222e ("selftests, x86: Rework x86 target architecture detection") >>> Reported-by: "kernelci.org bot" <bot@kernelci.org> >>> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com> >>> --- >>> tools/testing/selftests/x86/check_cc.sh | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/tools/testing/selftests/x86/check_cc.sh b/tools/testing/selftests/x86/check_cc.sh >>> index 3e2089c8cf549..aff2c15018b53 100755 >>> --- a/tools/testing/selftests/x86/check_cc.sh >>> +++ b/tools/testing/selftests/x86/check_cc.sh >>> @@ -7,7 +7,7 @@ CC="$1" >>> TESTPROG="$2" >>> shift 2 >>> >>> -if "$CC" -o /dev/null "$TESTPROG" -O0 "$@" 2>/dev/null; then >>> +if $CC -o /dev/null "$TESTPROG" -O0 "$@" 2>/dev/null; then >>> echo 1 >>> else >>> echo 0 >>> >> >> The intent is testing if $CC is set. Does this change work when >> $CC is not set? > > More by luck than judgement. Before and after. > If $CC might be empty you probably want: > > [ -n "$CC" ] && { echo 0; return; } > > The subject is also wrong. Should be "allow field splitting' of ${CC}. > (no brace or curly braces, not round ones.) > Good points. It would be good enhancement to add the check - since the current logic doesn't handle the null CC thanks, -- Shuah
On 2/11/22 10:13 PM, Shuah Khan wrote: > On 2/11/22 9:47 AM, David Laight wrote: >> From: Shuah Khan >>> Sent: 10 February 2022 20:52 >>> >>> On 2/10/22 12:06 PM, Muhammad Usama Anjum wrote: >>>> CC can have multiple sub-strings like "ccache gcc". Erorr pops up if >>>> it is treated as single string and double quote are used around it. >>>> This can be fixed by removing the quotes and not treating CC a single >>>> string. >>>> >>>> Fixes: e9886ace222e ("selftests, x86: Rework x86 target architecture >>>> detection") >>>> Reported-by: "kernelci.org bot" <bot@kernelci.org> >>>> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com> >>>> --- >>>> tools/testing/selftests/x86/check_cc.sh | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/tools/testing/selftests/x86/check_cc.sh >>>> b/tools/testing/selftests/x86/check_cc.sh >>>> index 3e2089c8cf549..aff2c15018b53 100755 >>>> --- a/tools/testing/selftests/x86/check_cc.sh >>>> +++ b/tools/testing/selftests/x86/check_cc.sh >>>> @@ -7,7 +7,7 @@ CC="$1" >>>> TESTPROG="$2" >>>> shift 2 >>>> >>>> -if "$CC" -o /dev/null "$TESTPROG" -O0 "$@" 2>/dev/null; then >>>> +if $CC -o /dev/null "$TESTPROG" -O0 "$@" 2>/dev/null; then >>>> echo 1 >>>> else >>>> echo 0 >>>> >>> >>> The intent is testing if $CC is set. Does this change work when >>> $CC is not set? >> >> More by luck than judgement. Before and after. >> If $CC might be empty you probably want: >> >> [ -n "$CC" ] && { echo 0; return; } >> >> The subject is also wrong. Should be "allow field splitting' of ${CC}. >> (no brace or curly braces, not round ones.) >> > > Good points. It would be good enhancement to add the check - since the > current logic doesn't handle the null CC > I'll send a V2. > thanks, > -- Shuah
diff --git a/tools/testing/selftests/x86/check_cc.sh b/tools/testing/selftests/x86/check_cc.sh index 3e2089c8cf549..aff2c15018b53 100755 --- a/tools/testing/selftests/x86/check_cc.sh +++ b/tools/testing/selftests/x86/check_cc.sh @@ -7,7 +7,7 @@ CC="$1" TESTPROG="$2" shift 2 -if "$CC" -o /dev/null "$TESTPROG" -O0 "$@" 2>/dev/null; then +if $CC -o /dev/null "$TESTPROG" -O0 "$@" 2>/dev/null; then echo 1 else echo 0
CC can have multiple sub-strings like "ccache gcc". Erorr pops up if it is treated as single string and double quote are used around it. This can be fixed by removing the quotes and not treating CC a single string. Fixes: e9886ace222e ("selftests, x86: Rework x86 target architecture detection") Reported-by: "kernelci.org bot" <bot@kernelci.org> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com> --- tools/testing/selftests/x86/check_cc.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)