qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] tests: Handle $RANDOM not being supported by the shell
@ 2017-07-14 10:45 Peter Maydell
  2017-07-14 11:27 ` Kamil Rytarowski
                   ` (5 more replies)
  0 siblings, 6 replies; 9+ messages in thread
From: Peter Maydell @ 2017-07-14 10:45 UTC (permalink / raw)
  To: qemu-devel; +Cc: patches, Eric Blake, Kamil Rytarowski

In various places in our test makefiles and scripts we use the
shell $RANDOM to create a random number. This is a bash
specific extension, and doesn't work on other shells.
With dash the shell doesn't complain, it just effectively
always evaluates $RANDOM to 0:
  echo $((RANDOM + 32768))     => 32768

However, on NetBSD the shell will complain:
  "-sh: arith: syntax error: "RANDOM + 32768"

which means that "make check" fails.

Switch to using "${RANDOM:-0}" instead of $RANDOM,
which will portably either give us a random number or zero.
This means that on non-bash shells we don't get such
good test coverage via the MALLOC_PERTURB_ setting, but
we were already in that situation for non-bash shells.

Our only other uses of $RANDOM (in tests/qemu-iotests/check
and tests/qemu-iotests/162) are in shell scripts which use
a #!/bin/bash line so they are always run under bash.

Suggested-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 tests/Makefile.include | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 6d6cb74..f6310d2 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -826,7 +826,7 @@ $(patsubst %, check-qtest-%, $(QTEST_TARGETS)): check-qtest-%: $(check-qtest-y)
 	$(if $(CONFIG_GCOV),@rm -f *.gcda */*.gcda */*/*.gcda */*/*/*.gcda,)
 	$(call quiet-command,QTEST_QEMU_BINARY=$*-softmmu/qemu-system-$* \
 		QTEST_QEMU_IMG=qemu-img$(EXESUF) \
-		MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$((RANDOM % 255 + 1))} \
+		MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$(( $${RANDOM:-0} % 255 + 1))} \
 		gtester $(GTESTER_OPTIONS) -m=$(SPEED) $(check-qtest-$*-y) $(check-qtest-generic-y),"GTESTER","$@")
 	$(if $(CONFIG_GCOV),@for f in $(gcov-files-$*-y) $(gcov-files-generic-y); do \
 	  echo Gcov report for $$f:;\
@@ -837,7 +837,7 @@ $(patsubst %, check-qtest-%, $(QTEST_TARGETS)): check-qtest-%: $(check-qtest-y)
 $(patsubst %, check-%, $(check-unit-y)): check-%: %
 	$(if $(CONFIG_GCOV),@rm -f *.gcda */*.gcda */*/*.gcda */*/*/*.gcda,)
 	$(call quiet-command, \
-		MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$((RANDOM % 255 + 1))} \
+		MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$(( $${RANDOM:-0} % 255 + 1))} \
 		gtester $(GTESTER_OPTIONS) -m=$(SPEED) $*,"GTESTER","$*")
 	$(if $(CONFIG_GCOV),@for f in $(gcov-files-$(subst tests/,,$*)-y) $(gcov-files-generic-y); do \
 	  echo Gcov report for $$f:;\
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH] tests: Handle $RANDOM not being supported by the shell
  2017-07-14 10:45 [Qemu-devel] [PATCH] tests: Handle $RANDOM not being supported by the shell Peter Maydell
@ 2017-07-14 11:27 ` Kamil Rytarowski
  2017-07-14 11:50   ` Kamil Rytarowski
  2017-07-14 12:02   ` Eric Blake
  2017-07-14 11:41 ` Fam Zheng
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 9+ messages in thread
From: Kamil Rytarowski @ 2017-07-14 11:27 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel; +Cc: patches, Eric Blake

[-- Attachment #1: Type: text/plain, Size: 2839 bytes --]

On 14.07.2017 12:45, Peter Maydell wrote:
> In various places in our test makefiles and scripts we use the
> shell $RANDOM to create a random number. This is a bash
> specific extension, and doesn't work on other shells.

This is supported on other shells like ksh (Korn Shell), but still as an
extension.

> With dash the shell doesn't complain, it just effectively
> always evaluates $RANDOM to 0:
>   echo $((RANDOM + 32768))     => 32768
> 
> However, on NetBSD the shell will complain:
>   "-sh: arith: syntax error: "RANDOM + 32768"
> 

I will make sure whether this behavior is correct in our sh(1).

> which means that "make check" fails.
> 
> Switch to using "${RANDOM:-0}" instead of $RANDOM,
> which will portably either give us a random number or zero.
> This means that on non-bash shells we don't get such
> good test coverage via the MALLOC_PERTURB_ setting, but
> we were already in that situation for non-bash shells.
> 
> Our only other uses of $RANDOM (in tests/qemu-iotests/check
> and tests/qemu-iotests/162) are in shell scripts which use
> a #!/bin/bash line so they are always run under bash.
> 
> Suggested-by: Eric Blake <eblake@redhat.com>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Kamil Rytarowski <n54@gmx.com>

> ---
>  tests/Makefile.include | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index 6d6cb74..f6310d2 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -826,7 +826,7 @@ $(patsubst %, check-qtest-%, $(QTEST_TARGETS)): check-qtest-%: $(check-qtest-y)
>  	$(if $(CONFIG_GCOV),@rm -f *.gcda */*.gcda */*/*.gcda */*/*/*.gcda,)
>  	$(call quiet-command,QTEST_QEMU_BINARY=$*-softmmu/qemu-system-$* \
>  		QTEST_QEMU_IMG=qemu-img$(EXESUF) \
> -		MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$((RANDOM % 255 + 1))} \
> +		MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$(( $${RANDOM:-0} % 255 + 1))} \
>  		gtester $(GTESTER_OPTIONS) -m=$(SPEED) $(check-qtest-$*-y) $(check-qtest-generic-y),"GTESTER","$@")
>  	$(if $(CONFIG_GCOV),@for f in $(gcov-files-$*-y) $(gcov-files-generic-y); do \
>  	  echo Gcov report for $$f:;\
> @@ -837,7 +837,7 @@ $(patsubst %, check-qtest-%, $(QTEST_TARGETS)): check-qtest-%: $(check-qtest-y)
>  $(patsubst %, check-%, $(check-unit-y)): check-%: %
>  	$(if $(CONFIG_GCOV),@rm -f *.gcda */*.gcda */*/*.gcda */*/*/*.gcda,)
>  	$(call quiet-command, \
> -		MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$((RANDOM % 255 + 1))} \
> +		MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$(( $${RANDOM:-0} % 255 + 1))} \
>  		gtester $(GTESTER_OPTIONS) -m=$(SPEED) $*,"GTESTER","$*")
>  	$(if $(CONFIG_GCOV),@for f in $(gcov-files-$(subst tests/,,$*)-y) $(gcov-files-generic-y); do \
>  	  echo Gcov report for $$f:;\
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH] tests: Handle $RANDOM not being supported by the shell
  2017-07-14 10:45 [Qemu-devel] [PATCH] tests: Handle $RANDOM not being supported by the shell Peter Maydell
  2017-07-14 11:27 ` Kamil Rytarowski
@ 2017-07-14 11:41 ` Fam Zheng
  2017-07-14 12:03 ` Eric Blake
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Fam Zheng @ 2017-07-14 11:41 UTC (permalink / raw)
  To: Peter Maydell; +Cc: qemu-devel, Kamil Rytarowski, patches

On Fri, 07/14 11:45, Peter Maydell wrote:
> In various places in our test makefiles and scripts we use the
> shell $RANDOM to create a random number. This is a bash
> specific extension, and doesn't work on other shells.
> With dash the shell doesn't complain, it just effectively
> always evaluates $RANDOM to 0:
>   echo $((RANDOM + 32768))     => 32768
> 
> However, on NetBSD the shell will complain:
>   "-sh: arith: syntax error: "RANDOM + 32768"
> 
> which means that "make check" fails.
> 
> Switch to using "${RANDOM:-0}" instead of $RANDOM,
> which will portably either give us a random number or zero.
> This means that on non-bash shells we don't get such
> good test coverage via the MALLOC_PERTURB_ setting, but
> we were already in that situation for non-bash shells.
> 
> Our only other uses of $RANDOM (in tests/qemu-iotests/check
> and tests/qemu-iotests/162) are in shell scripts which use
> a #!/bin/bash line so they are always run under bash.
> 
> Suggested-by: Eric Blake <eblake@redhat.com>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  tests/Makefile.include | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index 6d6cb74..f6310d2 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -826,7 +826,7 @@ $(patsubst %, check-qtest-%, $(QTEST_TARGETS)): check-qtest-%: $(check-qtest-y)
>  	$(if $(CONFIG_GCOV),@rm -f *.gcda */*.gcda */*/*.gcda */*/*/*.gcda,)
>  	$(call quiet-command,QTEST_QEMU_BINARY=$*-softmmu/qemu-system-$* \
>  		QTEST_QEMU_IMG=qemu-img$(EXESUF) \
> -		MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$((RANDOM % 255 + 1))} \
> +		MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$(( $${RANDOM:-0} % 255 + 1))} \
>  		gtester $(GTESTER_OPTIONS) -m=$(SPEED) $(check-qtest-$*-y) $(check-qtest-generic-y),"GTESTER","$@")
>  	$(if $(CONFIG_GCOV),@for f in $(gcov-files-$*-y) $(gcov-files-generic-y); do \
>  	  echo Gcov report for $$f:;\
> @@ -837,7 +837,7 @@ $(patsubst %, check-qtest-%, $(QTEST_TARGETS)): check-qtest-%: $(check-qtest-y)
>  $(patsubst %, check-%, $(check-unit-y)): check-%: %
>  	$(if $(CONFIG_GCOV),@rm -f *.gcda */*.gcda */*/*.gcda */*/*/*.gcda,)
>  	$(call quiet-command, \
> -		MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$((RANDOM % 255 + 1))} \
> +		MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$(( $${RANDOM:-0} % 255 + 1))} \

The whitespaces before $${RANDOM:-0} look a bit strange (unusual and unpaired).
Otherwise looks good. (I guess it provides a tiny bit of more readability given
how many non-whitespaces are already there before it.)

Regardlessly:

Reviewed-by: Fam Zheng <famz@redhat.com>

>  		gtester $(GTESTER_OPTIONS) -m=$(SPEED) $*,"GTESTER","$*")
>  	$(if $(CONFIG_GCOV),@for f in $(gcov-files-$(subst tests/,,$*)-y) $(gcov-files-generic-y); do \
>  	  echo Gcov report for $$f:;\
> -- 
> 2.7.4
> 
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH] tests: Handle $RANDOM not being supported by the shell
  2017-07-14 11:27 ` Kamil Rytarowski
@ 2017-07-14 11:50   ` Kamil Rytarowski
  2017-07-14 12:02   ` Eric Blake
  1 sibling, 0 replies; 9+ messages in thread
From: Kamil Rytarowski @ 2017-07-14 11:50 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel; +Cc: patches, Eric Blake

[-- Attachment #1: Type: text/plain, Size: 3043 bytes --]

On 14.07.2017 13:27, Kamil Rytarowski wrote:
> On 14.07.2017 12:45, Peter Maydell wrote:
>> In various places in our test makefiles and scripts we use the
>> shell $RANDOM to create a random number. This is a bash
>> specific extension, and doesn't work on other shells.
> 
> This is supported on other shells like ksh (Korn Shell), but still as an
> extension.
> 
>> With dash the shell doesn't complain, it just effectively
>> always evaluates $RANDOM to 0:
>>   echo $((RANDOM + 32768))     => 32768
>>
>> However, on NetBSD the shell will complain:
>>   "-sh: arith: syntax error: "RANDOM + 32768"
>>
> 
> I will make sure whether this behavior is correct in our sh(1).
> 

This has been altered in NetBSD-current (+ 8.0BETA), sh(1) behaves like
dash.

>> which means that "make check" fails.
>>
>> Switch to using "${RANDOM:-0}" instead of $RANDOM,
>> which will portably either give us a random number or zero.
>> This means that on non-bash shells we don't get such
>> good test coverage via the MALLOC_PERTURB_ setting, but
>> we were already in that situation for non-bash shells.
>>
>> Our only other uses of $RANDOM (in tests/qemu-iotests/check
>> and tests/qemu-iotests/162) are in shell scripts which use
>> a #!/bin/bash line so they are always run under bash.
>>
>> Suggested-by: Eric Blake <eblake@redhat.com>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> 
> Reviewed-by: Kamil Rytarowski <n54@gmx.com>
> 
>> ---
>>  tests/Makefile.include | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/Makefile.include b/tests/Makefile.include
>> index 6d6cb74..f6310d2 100644
>> --- a/tests/Makefile.include
>> +++ b/tests/Makefile.include
>> @@ -826,7 +826,7 @@ $(patsubst %, check-qtest-%, $(QTEST_TARGETS)): check-qtest-%: $(check-qtest-y)
>>  	$(if $(CONFIG_GCOV),@rm -f *.gcda */*.gcda */*/*.gcda */*/*/*.gcda,)
>>  	$(call quiet-command,QTEST_QEMU_BINARY=$*-softmmu/qemu-system-$* \
>>  		QTEST_QEMU_IMG=qemu-img$(EXESUF) \
>> -		MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$((RANDOM % 255 + 1))} \
>> +		MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$(( $${RANDOM:-0} % 255 + 1))} \
>>  		gtester $(GTESTER_OPTIONS) -m=$(SPEED) $(check-qtest-$*-y) $(check-qtest-generic-y),"GTESTER","$@")
>>  	$(if $(CONFIG_GCOV),@for f in $(gcov-files-$*-y) $(gcov-files-generic-y); do \
>>  	  echo Gcov report for $$f:;\
>> @@ -837,7 +837,7 @@ $(patsubst %, check-qtest-%, $(QTEST_TARGETS)): check-qtest-%: $(check-qtest-y)
>>  $(patsubst %, check-%, $(check-unit-y)): check-%: %
>>  	$(if $(CONFIG_GCOV),@rm -f *.gcda */*.gcda */*/*.gcda */*/*/*.gcda,)
>>  	$(call quiet-command, \
>> -		MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$((RANDOM % 255 + 1))} \
>> +		MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$(( $${RANDOM:-0} % 255 + 1))} \
>>  		gtester $(GTESTER_OPTIONS) -m=$(SPEED) $*,"GTESTER","$*")
>>  	$(if $(CONFIG_GCOV),@for f in $(gcov-files-$(subst tests/,,$*)-y) $(gcov-files-generic-y); do \
>>  	  echo Gcov report for $$f:;\
>>
> 
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH] tests: Handle $RANDOM not being supported by the shell
  2017-07-14 11:27 ` Kamil Rytarowski
  2017-07-14 11:50   ` Kamil Rytarowski
@ 2017-07-14 12:02   ` Eric Blake
  1 sibling, 0 replies; 9+ messages in thread
From: Eric Blake @ 2017-07-14 12:02 UTC (permalink / raw)
  To: Kamil Rytarowski, Peter Maydell, qemu-devel; +Cc: patches

[-- Attachment #1: Type: text/plain, Size: 1209 bytes --]

On 07/14/2017 06:27 AM, Kamil Rytarowski wrote:
> On 14.07.2017 12:45, Peter Maydell wrote:
>> In various places in our test makefiles and scripts we use the
>> shell $RANDOM to create a random number. This is a bash
>> specific extension, and doesn't work on other shells.
> 
> This is supported on other shells like ksh (Korn Shell), but still as an
> extension.
> 
>> With dash the shell doesn't complain, it just effectively
>> always evaluates $RANDOM to 0:
>>   echo $((RANDOM + 32768))     => 32768
>>
>> However, on NetBSD the shell will complain:
>>   "-sh: arith: syntax error: "RANDOM + 32768"
>>
> 
> I will make sure whether this behavior is correct in our sh(1).

The question is what your shell does for:

unset foo
echo $(( foo % 255 ))

if it reliably prints 0, then it would do the same when RANDOM is an
undefined (and not a magic) variable.  Presumably, where sh is going
wrong is that it is treating an undefined variable not as 0, but as a
syntax error, and you'd get that behavior regardless of whether RANDOM
is in the mix.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH] tests: Handle $RANDOM not being supported by the shell
  2017-07-14 10:45 [Qemu-devel] [PATCH] tests: Handle $RANDOM not being supported by the shell Peter Maydell
  2017-07-14 11:27 ` Kamil Rytarowski
  2017-07-14 11:41 ` Fam Zheng
@ 2017-07-14 12:03 ` Eric Blake
  2017-07-17  9:43 ` Stefan Hajnoczi
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Eric Blake @ 2017-07-14 12:03 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel; +Cc: patches, Kamil Rytarowski

[-- Attachment #1: Type: text/plain, Size: 1259 bytes --]

On 07/14/2017 05:45 AM, Peter Maydell wrote:
> In various places in our test makefiles and scripts we use the
> shell $RANDOM to create a random number. This is a bash
> specific extension, and doesn't work on other shells.

As mentioned elsewhere, you could reword this to "this is an extension
in bash and some other shells, but not universal".  But I'm also okay
leaving it untouched.

> Switch to using "${RANDOM:-0}" instead of $RANDOM,
> which will portably either give us a random number or zero.
> This means that on non-bash shells we don't get such
> good test coverage via the MALLOC_PERTURB_ setting, but
> we were already in that situation for non-bash shells.
> 
> Our only other uses of $RANDOM (in tests/qemu-iotests/check
> and tests/qemu-iotests/162) are in shell scripts which use
> a #!/bin/bash line so they are always run under bash.
> 
> Suggested-by: Eric Blake <eblake@redhat.com>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  tests/Makefile.include | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH] tests: Handle $RANDOM not being supported by the shell
  2017-07-14 10:45 [Qemu-devel] [PATCH] tests: Handle $RANDOM not being supported by the shell Peter Maydell
                   ` (2 preceding siblings ...)
  2017-07-14 12:03 ` Eric Blake
@ 2017-07-17  9:43 ` Stefan Hajnoczi
  2017-07-20 11:26 ` Markus Armbruster
  2017-07-20 15:34 ` Peter Maydell
  5 siblings, 0 replies; 9+ messages in thread
From: Stefan Hajnoczi @ 2017-07-17  9:43 UTC (permalink / raw)
  To: Peter Maydell; +Cc: qemu-devel, Kamil Rytarowski, patches

[-- Attachment #1: Type: text/plain, Size: 1268 bytes --]

On Fri, Jul 14, 2017 at 11:45:17AM +0100, Peter Maydell wrote:
> In various places in our test makefiles and scripts we use the
> shell $RANDOM to create a random number. This is a bash
> specific extension, and doesn't work on other shells.
> With dash the shell doesn't complain, it just effectively
> always evaluates $RANDOM to 0:
>   echo $((RANDOM + 32768))     => 32768
> 
> However, on NetBSD the shell will complain:
>   "-sh: arith: syntax error: "RANDOM + 32768"
> 
> which means that "make check" fails.
> 
> Switch to using "${RANDOM:-0}" instead of $RANDOM,
> which will portably either give us a random number or zero.
> This means that on non-bash shells we don't get such
> good test coverage via the MALLOC_PERTURB_ setting, but
> we were already in that situation for non-bash shells.
> 
> Our only other uses of $RANDOM (in tests/qemu-iotests/check
> and tests/qemu-iotests/162) are in shell scripts which use
> a #!/bin/bash line so they are always run under bash.
> 
> Suggested-by: Eric Blake <eblake@redhat.com>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  tests/Makefile.include | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH] tests: Handle $RANDOM not being supported by the shell
  2017-07-14 10:45 [Qemu-devel] [PATCH] tests: Handle $RANDOM not being supported by the shell Peter Maydell
                   ` (3 preceding siblings ...)
  2017-07-17  9:43 ` Stefan Hajnoczi
@ 2017-07-20 11:26 ` Markus Armbruster
  2017-07-20 15:34 ` Peter Maydell
  5 siblings, 0 replies; 9+ messages in thread
From: Markus Armbruster @ 2017-07-20 11:26 UTC (permalink / raw)
  To: Peter Maydell; +Cc: qemu-devel, Kamil Rytarowski, patches

Peter Maydell <peter.maydell@linaro.org> writes:

> In various places in our test makefiles and scripts we use the
> shell $RANDOM to create a random number. This is a bash
> specific extension, and doesn't work on other shells.
> With dash the shell doesn't complain, it just effectively
> always evaluates $RANDOM to 0:
>   echo $((RANDOM + 32768))     => 32768
>
> However, on NetBSD the shell will complain:
>   "-sh: arith: syntax error: "RANDOM + 32768"
>
> which means that "make check" fails.
>
> Switch to using "${RANDOM:-0}" instead of $RANDOM,
> which will portably either give us a random number or zero.

Golden opportunity to implement https://www.xkcd.com/221/

> This means that on non-bash shells we don't get such
> good test coverage via the MALLOC_PERTURB_ setting, but
> we were already in that situation for non-bash shells.
>
> Our only other uses of $RANDOM (in tests/qemu-iotests/check
> and tests/qemu-iotests/162) are in shell scripts which use
> a #!/bin/bash line so they are always run under bash.
>
> Suggested-by: Eric Blake <eblake@redhat.com>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  tests/Makefile.include | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index 6d6cb74..f6310d2 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -826,7 +826,7 @@ $(patsubst %, check-qtest-%, $(QTEST_TARGETS)): check-qtest-%: $(check-qtest-y)
>  	$(if $(CONFIG_GCOV),@rm -f *.gcda */*.gcda */*/*.gcda */*/*/*.gcda,)
>  	$(call quiet-command,QTEST_QEMU_BINARY=$*-softmmu/qemu-system-$* \
>  		QTEST_QEMU_IMG=qemu-img$(EXESUF) \
> -		MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$((RANDOM % 255 + 1))} \
> +		MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$(( $${RANDOM:-0} % 255 + 1))} \
>  		gtester $(GTESTER_OPTIONS) -m=$(SPEED) $(check-qtest-$*-y) $(check-qtest-generic-y),"GTESTER","$@")
>  	$(if $(CONFIG_GCOV),@for f in $(gcov-files-$*-y) $(gcov-files-generic-y); do \
>  	  echo Gcov report for $$f:;\
> @@ -837,7 +837,7 @@ $(patsubst %, check-qtest-%, $(QTEST_TARGETS)): check-qtest-%: $(check-qtest-y)
>  $(patsubst %, check-%, $(check-unit-y)): check-%: %
>  	$(if $(CONFIG_GCOV),@rm -f *.gcda */*.gcda */*/*.gcda */*/*/*.gcda,)
>  	$(call quiet-command, \
> -		MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$((RANDOM % 255 + 1))} \
> +		MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$(( $${RANDOM:-0} % 255 + 1))} \
>  		gtester $(GTESTER_OPTIONS) -m=$(SPEED) $*,"GTESTER","$*")
>  	$(if $(CONFIG_GCOV),@for f in $(gcov-files-$(subst tests/,,$*)-y) $(gcov-files-generic-y); do \
>  	  echo Gcov report for $$f:;\

A possible alternative might be putting

    export RANDOM = 4    # chosen by a fair dice roll

into the makefile.

There's another occurence in tests/qemu-iotests/162.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH] tests: Handle $RANDOM not being supported by the shell
  2017-07-14 10:45 [Qemu-devel] [PATCH] tests: Handle $RANDOM not being supported by the shell Peter Maydell
                   ` (4 preceding siblings ...)
  2017-07-20 11:26 ` Markus Armbruster
@ 2017-07-20 15:34 ` Peter Maydell
  5 siblings, 0 replies; 9+ messages in thread
From: Peter Maydell @ 2017-07-20 15:34 UTC (permalink / raw)
  To: QEMU Developers; +Cc: Kamil Rytarowski, patches@linaro.org

On 14 July 2017 at 11:45, Peter Maydell <peter.maydell@linaro.org> wrote:
> In various places in our test makefiles and scripts we use the
> shell $RANDOM to create a random number. This is a bash
> specific extension, and doesn't work on other shells.
> With dash the shell doesn't complain, it just effectively
> always evaluates $RANDOM to 0:
>   echo $((RANDOM + 32768))     => 32768
>
> However, on NetBSD the shell will complain:
>   "-sh: arith: syntax error: "RANDOM + 32768"
>
> which means that "make check" fails.
>
> Switch to using "${RANDOM:-0}" instead of $RANDOM,
> which will portably either give us a random number or zero.
> This means that on non-bash shells we don't get such
> good test coverage via the MALLOC_PERTURB_ setting, but
> we were already in that situation for non-bash shells.
>
> Our only other uses of $RANDOM (in tests/qemu-iotests/check
> and tests/qemu-iotests/162) are in shell scripts which use
> a #!/bin/bash line so they are always run under bash.
>
> Suggested-by: Eric Blake <eblake@redhat.com>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  tests/Makefile.include | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>

Applied, thanks.

-- PMM

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2017-07-20 15:35 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-07-14 10:45 [Qemu-devel] [PATCH] tests: Handle $RANDOM not being supported by the shell Peter Maydell
2017-07-14 11:27 ` Kamil Rytarowski
2017-07-14 11:50   ` Kamil Rytarowski
2017-07-14 12:02   ` Eric Blake
2017-07-14 11:41 ` Fam Zheng
2017-07-14 12:03 ` Eric Blake
2017-07-17  9:43 ` Stefan Hajnoczi
2017-07-20 11:26 ` Markus Armbruster
2017-07-20 15:34 ` Peter Maydell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).