[PATCH] fuzz: fix unbound variable in build.sh

[PATCH] fuzz: fix unbound variable in build.sh

[Alexander Bulekov]

/src/build.sh: line 76: GITLAB_CI: unbound variable
Fix that.

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
---

This change is in preparation to revert:
7602748c ("qemu: manually build glib (#5919)") on OSS-Fuzz.
Reverting as-is produces an unbound variable complaint when we try to
build the fuzzers in the OSS-Fuzz container.

 scripts/oss-fuzz/build.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/oss-fuzz/build.sh b/scripts/oss-fuzz/build.sh
index 98b56e0521..5ddc769c9c 100755
--- a/scripts/oss-fuzz/build.sh
+++ b/scripts/oss-fuzz/build.sh
@@ -73,7 +73,7 @@ if ! make "-j$(nproc)" qemu-fuzz-i386; then
           "\nFor example: CC=clang CXX=clang++ $0"
 fi
 
-if [ "$GITLAB_CI" != "true" ]; then
+if [ -z ${GITLAB_CI+x} ]; then
     for i in $(ldd ./qemu-fuzz-i386 | cut -f3 -d' '); do
         cp "$i" "$DEST_DIR/lib/"
     done
-- 
2.30.2

Re: [PATCH] fuzz: fix unbound variable in build.sh

[Darren Kenny]

On Tuesday, 2021-09-07 at 07:08:41 -04, Alexander Bulekov wrote:
> /src/build.sh: line 76: GITLAB_CI: unbound variable
> Fix that.
>
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>

Reviewed-by: Darren Kenny <darren.kenny@oracle.com>

> ---
>
> This change is in preparation to revert:
> 7602748c ("qemu: manually build glib (#5919)") on OSS-Fuzz.
> Reverting as-is produces an unbound variable complaint when we try to
> build the fuzzers in the OSS-Fuzz container.
>
>  scripts/oss-fuzz/build.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/oss-fuzz/build.sh b/scripts/oss-fuzz/build.sh
> index 98b56e0521..5ddc769c9c 100755
> --- a/scripts/oss-fuzz/build.sh
> +++ b/scripts/oss-fuzz/build.sh
> @@ -73,7 +73,7 @@ if ! make "-j$(nproc)" qemu-fuzz-i386; then
>            "\nFor example: CC=clang CXX=clang++ $0"
>  fi
>  
> -if [ "$GITLAB_CI" != "true" ]; then
> +if [ -z ${GITLAB_CI+x} ]; then
>      for i in $(ldd ./qemu-fuzz-i386 | cut -f3 -d' '); do
>          cp "$i" "$DEST_DIR/lib/"
>      done
> -- 
> 2.30.2

Re: [PATCH] fuzz: fix unbound variable in build.sh

[Thomas Huth]

On 07/09/2021 13.08, Alexander Bulekov wrote:
> /src/build.sh: line 76: GITLAB_CI: unbound variable
> Fix that.
> 
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> ---
> 
> This change is in preparation to revert:
> 7602748c ("qemu: manually build glib (#5919)") on OSS-Fuzz.
> Reverting as-is produces an unbound variable complaint when we try to
> build the fuzzers in the OSS-Fuzz container.
> 
>   scripts/oss-fuzz/build.sh | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/oss-fuzz/build.sh b/scripts/oss-fuzz/build.sh
> index 98b56e0521..5ddc769c9c 100755
> --- a/scripts/oss-fuzz/build.sh
> +++ b/scripts/oss-fuzz/build.sh
> @@ -73,7 +73,7 @@ if ! make "-j$(nproc)" qemu-fuzz-i386; then
>             "\nFor example: CC=clang CXX=clang++ $0"
>   fi
>   
> -if [ "$GITLAB_CI" != "true" ]; then
> +if [ -z ${GITLAB_CI+x} ]; then

My bash-foo is really not the best, but shouldn't there be a colon in there, 
i.e. ${GITLAB_CI:+x} ?

  Thomas


>       for i in $(ldd ./qemu-fuzz-i386 | cut -f3 -d' '); do
>           cp "$i" "$DEST_DIR/lib/"
>       done
> 

Re: [PATCH] fuzz: fix unbound variable in build.sh

[Alexander Bulekov]

On 210907 1432, Thomas Huth wrote:
> On 07/09/2021 13.08, Alexander Bulekov wrote:
> > /src/build.sh: line 76: GITLAB_CI: unbound variable
> > Fix that.
> > 
> > Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> > ---
> > 
> > This change is in preparation to revert:
> > 7602748c ("qemu: manually build glib (#5919)") on OSS-Fuzz.
> > Reverting as-is produces an unbound variable complaint when we try to
> > build the fuzzers in the OSS-Fuzz container.
> > 
> >   scripts/oss-fuzz/build.sh | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/scripts/oss-fuzz/build.sh b/scripts/oss-fuzz/build.sh
> > index 98b56e0521..5ddc769c9c 100755
> > --- a/scripts/oss-fuzz/build.sh
> > +++ b/scripts/oss-fuzz/build.sh
> > @@ -73,7 +73,7 @@ if ! make "-j$(nproc)" qemu-fuzz-i386; then
> >             "\nFor example: CC=clang CXX=clang++ $0"
> >   fi
> > -if [ "$GITLAB_CI" != "true" ]; then
> > +if [ -z ${GITLAB_CI+x} ]; then
> 
> My bash-foo is really not the best, but shouldn't there be a colon in there,
> i.e. ${GITLAB_CI:+x} ?

I think the difference is that GITLAB_CI+x only checks if GITLAB_CI is
set, while GITLAB_CI:+x checks that it is set and non-null.
https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_06_02

I don't think that makes much of a difference here.

-Alex
> 
>  Thomas
> 
> 
> >       for i in $(ldd ./qemu-fuzz-i386 | cut -f3 -d' '); do
> >           cp "$i" "$DEST_DIR/lib/"
> >       done
> > 
> 

Re: [PATCH] fuzz: fix unbound variable in build.sh

[Thomas Huth]

On 07/09/2021 14.51, Alexander Bulekov wrote:
> On 210907 1432, Thomas Huth wrote:
>> On 07/09/2021 13.08, Alexander Bulekov wrote:
>>> /src/build.sh: line 76: GITLAB_CI: unbound variable
>>> Fix that.
>>>
>>> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
>>> ---
>>>
>>> This change is in preparation to revert:
>>> 7602748c ("qemu: manually build glib (#5919)") on OSS-Fuzz.
>>> Reverting as-is produces an unbound variable complaint when we try to
>>> build the fuzzers in the OSS-Fuzz container.
>>>
>>>    scripts/oss-fuzz/build.sh | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/scripts/oss-fuzz/build.sh b/scripts/oss-fuzz/build.sh
>>> index 98b56e0521..5ddc769c9c 100755
>>> --- a/scripts/oss-fuzz/build.sh
>>> +++ b/scripts/oss-fuzz/build.sh
>>> @@ -73,7 +73,7 @@ if ! make "-j$(nproc)" qemu-fuzz-i386; then
>>>              "\nFor example: CC=clang CXX=clang++ $0"
>>>    fi
>>> -if [ "$GITLAB_CI" != "true" ]; then
>>> +if [ -z ${GITLAB_CI+x} ]; then
>>
>> My bash-foo is really not the best, but shouldn't there be a colon in there,
>> i.e. ${GITLAB_CI:+x} ?
> 
> I think the difference is that GITLAB_CI+x only checks if GITLAB_CI is
> set, while GITLAB_CI:+x checks that it is set and non-null.
> https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_06_02
> 
> I don't think that makes much of a difference here.

TIL, and I agree that it does not make a difference here (and if it would, 
your variant is certainly better).

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH] fuzz: fix unbound variable in build.sh

[Paolo Bonzini]

On 07/09/21 13:08, Alexander Bulekov wrote:
>   
> -if [ "$GITLAB_CI" != "true" ]; then
> +if [ -z ${GITLAB_CI+x} ]; then

I would slightly prefer to have "${GITLAB_CI+x}", since "test" in 
general doesn't like parameters that go away:

$ [ = abc ]
bash: [: =: unary operator expected

What you wrote however works, so it's okay.

Thanks,

Paolo

Re: [PATCH] fuzz: fix unbound variable in build.sh

[Darren Kenny]

On Wednesday, 2021-09-08 at 08:06:27 +02, Paolo Bonzini wrote:
> On 07/09/21 13:08, Alexander Bulekov wrote:
>>   
>> -if [ "$GITLAB_CI" != "true" ]; then
>> +if [ -z ${GITLAB_CI+x} ]; then
>
> I would slightly prefer to have "${GITLAB_CI+x}", since "test" in 
> general doesn't like parameters that go away:
>
> $ [ = abc ]
> bash: [: =: unary operator expected
>
> What you wrote however works, so it's okay.
>

If we are certain that the script is running a bash variant then we
really should be using the '[[ ... ]]' variant of test which doesn't
have that limitation since it can handle an empty or unset variable
correctly.

This doesn't appear to be the assumption though.

Thanks,

Darren.