kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH kvm-unit-tests 0/3] A few miscellaneous fixes
@ 2020-10-14 19:14 Andrew Jones
  2020-10-14 19:14 ` [PATCH kvm-unit-tests 1/3] lib/string: Fix getenv name matching Andrew Jones
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Andrew Jones @ 2020-10-14 19:14 UTC (permalink / raw)
  To: kvm; +Cc: pbonzini, thuth

While recently working on kvm-unit-tests I found a few bugs.
Posting the patches now since the series I'm working on may
not get merged for awhile.


Andrew Jones (3):
  lib/string: Fix getenv name matching
  scripts: Save rematch before calling out of for_each_unittest
  arm/arm64: Change dcache_line_size to ulong

 lib/arm/asm/processor.h   | 2 +-
 lib/arm/setup.c           | 2 +-
 lib/arm64/asm/processor.h | 2 +-
 lib/string.c              | 5 ++++-
 scripts/common.bash       | 4 +++-
 5 files changed, 10 insertions(+), 5 deletions(-)

-- 
2.26.2


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

* [PATCH kvm-unit-tests 1/3] lib/string: Fix getenv name matching
  2020-10-14 19:14 [PATCH kvm-unit-tests 0/3] A few miscellaneous fixes Andrew Jones
@ 2020-10-14 19:14 ` Andrew Jones
  2020-10-15  8:19   ` Thomas Huth
  2020-10-31 14:31   ` Paolo Bonzini
  2020-10-14 19:14 ` [PATCH kvm-unit-tests 2/3] scripts: Save rematch before calling out of for_each_unittest Andrew Jones
  2020-10-14 19:14 ` [PATCH kvm-unit-tests 3/3] arm/arm64: Change dcache_line_size to ulong Andrew Jones
  2 siblings, 2 replies; 10+ messages in thread
From: Andrew Jones @ 2020-10-14 19:14 UTC (permalink / raw)
  To: kvm; +Cc: pbonzini, thuth

Without confirming that the name length exactly matches too, then
the string comparison would return the same value for VAR* as for
VAR, when VAR came first in the environ array.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 lib/string.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/string.c b/lib/string.c
index 018dcc879516..3a0562120f12 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -171,10 +171,13 @@ extern char **environ;
 char *getenv(const char *name)
 {
     char **envp = environ, *delim;
+    int len;
 
     while (*envp) {
         delim = strchr(*envp, '=');
-        if (delim && strncmp(name, *envp, delim - *envp) == 0)
+        assert(delim);
+        len = delim - *envp;
+        if (strlen(name) == len && strncmp(name, *envp, len) == 0)
             return delim + 1;
         ++envp;
     }
-- 
2.26.2


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

* [PATCH kvm-unit-tests 2/3] scripts: Save rematch before calling out of for_each_unittest
  2020-10-14 19:14 [PATCH kvm-unit-tests 0/3] A few miscellaneous fixes Andrew Jones
  2020-10-14 19:14 ` [PATCH kvm-unit-tests 1/3] lib/string: Fix getenv name matching Andrew Jones
@ 2020-10-14 19:14 ` Andrew Jones
  2020-10-15  8:22   ` Thomas Huth
  2020-10-14 19:14 ` [PATCH kvm-unit-tests 3/3] arm/arm64: Change dcache_line_size to ulong Andrew Jones
  2 siblings, 1 reply; 10+ messages in thread
From: Andrew Jones @ 2020-10-14 19:14 UTC (permalink / raw)
  To: kvm; +Cc: pbonzini, thuth

If we don't save BASH_REMATCH before calling another function,
and that other function also uses [[...]], then we'll lose the
test.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 scripts/common.bash | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/scripts/common.bash b/scripts/common.bash
index a6044b7c6c35..7b983f7d6dd6 100644
--- a/scripts/common.bash
+++ b/scripts/common.bash
@@ -13,15 +13,17 @@ function for_each_unittest()
 	local check
 	local accel
 	local timeout
+	local rematch
 
 	exec {fd}<"$unittests"
 
 	while read -r -u $fd line; do
 		if [[ "$line" =~ ^\[(.*)\]$ ]]; then
+			rematch=${BASH_REMATCH[1]}
 			if [ -n "${testname}" ]; then
 				$(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
 			fi
-			testname=${BASH_REMATCH[1]}
+			testname=$rematch
 			smp=1
 			kernel=""
 			opts=""
-- 
2.26.2


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

* [PATCH kvm-unit-tests 3/3] arm/arm64: Change dcache_line_size to ulong
  2020-10-14 19:14 [PATCH kvm-unit-tests 0/3] A few miscellaneous fixes Andrew Jones
  2020-10-14 19:14 ` [PATCH kvm-unit-tests 1/3] lib/string: Fix getenv name matching Andrew Jones
  2020-10-14 19:14 ` [PATCH kvm-unit-tests 2/3] scripts: Save rematch before calling out of for_each_unittest Andrew Jones
@ 2020-10-14 19:14 ` Andrew Jones
  2020-10-15  8:30   ` Thomas Huth
  2 siblings, 1 reply; 10+ messages in thread
From: Andrew Jones @ 2020-10-14 19:14 UTC (permalink / raw)
  To: kvm; +Cc: pbonzini, thuth

dcache_line_size is treated like a long in assembly, so make it one.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 lib/arm/asm/processor.h   | 2 +-
 lib/arm/setup.c           | 2 +-
 lib/arm64/asm/processor.h | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/arm/asm/processor.h b/lib/arm/asm/processor.h
index e26ef89000a8..273366d1fe1c 100644
--- a/lib/arm/asm/processor.h
+++ b/lib/arm/asm/processor.h
@@ -89,6 +89,6 @@ static inline u32 get_ctr(void)
 	return read_sysreg(CTR);
 }
 
-extern u32 dcache_line_size;
+extern unsigned long dcache_line_size;
 
 #endif /* _ASMARM_PROCESSOR_H_ */
diff --git a/lib/arm/setup.c b/lib/arm/setup.c
index 78562e47c01c..ea714d064afa 100644
--- a/lib/arm/setup.c
+++ b/lib/arm/setup.c
@@ -42,7 +42,7 @@ static struct mem_region __initial_mem_regions[NR_INITIAL_MEM_REGIONS + 1];
 struct mem_region *mem_regions = __initial_mem_regions;
 phys_addr_t __phys_offset, __phys_end;
 
-u32 dcache_line_size;
+unsigned long dcache_line_size;
 
 int mpidr_to_cpu(uint64_t mpidr)
 {
diff --git a/lib/arm64/asm/processor.h b/lib/arm64/asm/processor.h
index 02665b84cc7e..6ee7c1260b6b 100644
--- a/lib/arm64/asm/processor.h
+++ b/lib/arm64/asm/processor.h
@@ -115,7 +115,7 @@ static inline u64 get_ctr(void)
 	return read_sysreg(ctr_el0);
 }
 
-extern u32 dcache_line_size;
+extern unsigned long dcache_line_size;
 
 #endif /* !__ASSEMBLY__ */
 #endif /* _ASMARM64_PROCESSOR_H_ */
-- 
2.26.2


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

* Re: [PATCH kvm-unit-tests 1/3] lib/string: Fix getenv name matching
  2020-10-14 19:14 ` [PATCH kvm-unit-tests 1/3] lib/string: Fix getenv name matching Andrew Jones
@ 2020-10-15  8:19   ` Thomas Huth
  2020-10-31 14:31   ` Paolo Bonzini
  1 sibling, 0 replies; 10+ messages in thread
From: Thomas Huth @ 2020-10-15  8:19 UTC (permalink / raw)
  To: Andrew Jones, kvm; +Cc: pbonzini

On 14/10/2020 21.14, Andrew Jones wrote:
> Without confirming that the name length exactly matches too, then
> the string comparison would return the same value for VAR* as for
> VAR, when VAR came first in the environ array.
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  lib/string.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/string.c b/lib/string.c
> index 018dcc879516..3a0562120f12 100644
> --- a/lib/string.c
> +++ b/lib/string.c
> @@ -171,10 +171,13 @@ extern char **environ;
>  char *getenv(const char *name)
>  {
>      char **envp = environ, *delim;
> +    int len;
>  
>      while (*envp) {
>          delim = strchr(*envp, '=');
> -        if (delim && strncmp(name, *envp, delim - *envp) == 0)
> +        assert(delim);
> +        len = delim - *envp;
> +        if (strlen(name) == len && strncmp(name, *envp, len) == 0)
>              return delim + 1;
>          ++envp;
>      }
> 

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


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

* Re: [PATCH kvm-unit-tests 2/3] scripts: Save rematch before calling out of for_each_unittest
  2020-10-14 19:14 ` [PATCH kvm-unit-tests 2/3] scripts: Save rematch before calling out of for_each_unittest Andrew Jones
@ 2020-10-15  8:22   ` Thomas Huth
  0 siblings, 0 replies; 10+ messages in thread
From: Thomas Huth @ 2020-10-15  8:22 UTC (permalink / raw)
  To: Andrew Jones, kvm; +Cc: pbonzini

On 14/10/2020 21.14, Andrew Jones wrote:
> If we don't save BASH_REMATCH before calling another function,
> and that other function also uses [[...]], then we'll lose the
> test.
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  scripts/common.bash | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/common.bash b/scripts/common.bash
> index a6044b7c6c35..7b983f7d6dd6 100644
> --- a/scripts/common.bash
> +++ b/scripts/common.bash
> @@ -13,15 +13,17 @@ function for_each_unittest()
>  	local check
>  	local accel
>  	local timeout
> +	local rematch
>  
>  	exec {fd}<"$unittests"
>  
>  	while read -r -u $fd line; do
>  		if [[ "$line" =~ ^\[(.*)\]$ ]]; then
> +			rematch=${BASH_REMATCH[1]}
>  			if [ -n "${testname}" ]; then
>  				$(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
>  			fi
> -			testname=${BASH_REMATCH[1]}
> +			testname=$rematch
>  			smp=1
>  			kernel=""
>  			opts=""
> 

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


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

* Re: [PATCH kvm-unit-tests 3/3] arm/arm64: Change dcache_line_size to ulong
  2020-10-14 19:14 ` [PATCH kvm-unit-tests 3/3] arm/arm64: Change dcache_line_size to ulong Andrew Jones
@ 2020-10-15  8:30   ` Thomas Huth
  2020-10-31 14:32     ` Paolo Bonzini
  0 siblings, 1 reply; 10+ messages in thread
From: Thomas Huth @ 2020-10-15  8:30 UTC (permalink / raw)
  To: Andrew Jones, kvm; +Cc: pbonzini

On 14/10/2020 21.14, Andrew Jones wrote:
> dcache_line_size is treated like a long in assembly, so make it one.
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  lib/arm/asm/processor.h   | 2 +-
>  lib/arm/setup.c           | 2 +-
>  lib/arm64/asm/processor.h | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/arm/asm/processor.h b/lib/arm/asm/processor.h
> index e26ef89000a8..273366d1fe1c 100644
> --- a/lib/arm/asm/processor.h
> +++ b/lib/arm/asm/processor.h
> @@ -89,6 +89,6 @@ static inline u32 get_ctr(void)
>  	return read_sysreg(CTR);
>  }
>  
> -extern u32 dcache_line_size;
> +extern unsigned long dcache_line_size;
>  
>  #endif /* _ASMARM_PROCESSOR_H_ */
> diff --git a/lib/arm/setup.c b/lib/arm/setup.c
> index 78562e47c01c..ea714d064afa 100644
> --- a/lib/arm/setup.c
> +++ b/lib/arm/setup.c
> @@ -42,7 +42,7 @@ static struct mem_region __initial_mem_regions[NR_INITIAL_MEM_REGIONS + 1];
>  struct mem_region *mem_regions = __initial_mem_regions;
>  phys_addr_t __phys_offset, __phys_end;
>  
> -u32 dcache_line_size;
> +unsigned long dcache_line_size;
>  
>  int mpidr_to_cpu(uint64_t mpidr)
>  {
> diff --git a/lib/arm64/asm/processor.h b/lib/arm64/asm/processor.h
> index 02665b84cc7e..6ee7c1260b6b 100644
> --- a/lib/arm64/asm/processor.h
> +++ b/lib/arm64/asm/processor.h
> @@ -115,7 +115,7 @@ static inline u64 get_ctr(void)
>  	return read_sysreg(ctr_el0);
>  }
>  
> -extern u32 dcache_line_size;
> +extern unsigned long dcache_line_size;
>  
>  #endif /* !__ASSEMBLY__ */
>  #endif /* _ASMARM64_PROCESSOR_H_ */
> 

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


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

* Re: [PATCH kvm-unit-tests 1/3] lib/string: Fix getenv name matching
  2020-10-14 19:14 ` [PATCH kvm-unit-tests 1/3] lib/string: Fix getenv name matching Andrew Jones
  2020-10-15  8:19   ` Thomas Huth
@ 2020-10-31 14:31   ` Paolo Bonzini
  1 sibling, 0 replies; 10+ messages in thread
From: Paolo Bonzini @ 2020-10-31 14:31 UTC (permalink / raw)
  To: Andrew Jones, kvm; +Cc: thuth

On 14/10/20 21:14, Andrew Jones wrote:
> +        if (strlen(name) == len && strncmp(name, *envp, len) == 0)
>              return delim + 1;
>          ++envp;
>      }
> -- 2.26.2


Slightly more efficient:

+        if (memcmp(name, *envp, len) == 0 && !name[len])

(strncmp is not needed since we know there's no NULL between *envp and
*envp+len, and after memcmp we know the same about the first len
characters of name).

Paolo


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

* Re: [PATCH kvm-unit-tests 3/3] arm/arm64: Change dcache_line_size to ulong
  2020-10-15  8:30   ` Thomas Huth
@ 2020-10-31 14:32     ` Paolo Bonzini
  2020-11-11 15:06       ` Andrew Jones
  0 siblings, 1 reply; 10+ messages in thread
From: Paolo Bonzini @ 2020-10-31 14:32 UTC (permalink / raw)
  To: Thomas Huth, Andrew Jones, kvm

On 15/10/20 10:30, Thomas Huth wrote:
> On 14/10/2020 21.14, Andrew Jones wrote:
>> dcache_line_size is treated like a long in assembly, so make it one.
>>
>> Signed-off-by: Andrew Jones <drjones@redhat.com>
>> ---
>>  lib/arm/asm/processor.h   | 2 +-
>>  lib/arm/setup.c           | 2 +-
>>  lib/arm64/asm/processor.h | 2 +-
>>  3 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/lib/arm/asm/processor.h b/lib/arm/asm/processor.h
>> index e26ef89000a8..273366d1fe1c 100644
>> --- a/lib/arm/asm/processor.h
>> +++ b/lib/arm/asm/processor.h
>> @@ -89,6 +89,6 @@ static inline u32 get_ctr(void)
>>  	return read_sysreg(CTR);
>>  }
>>  
>> -extern u32 dcache_line_size;
>> +extern unsigned long dcache_line_size;
>>  
>>  #endif /* _ASMARM_PROCESSOR_H_ */
>> diff --git a/lib/arm/setup.c b/lib/arm/setup.c
>> index 78562e47c01c..ea714d064afa 100644
>> --- a/lib/arm/setup.c
>> +++ b/lib/arm/setup.c
>> @@ -42,7 +42,7 @@ static struct mem_region __initial_mem_regions[NR_INITIAL_MEM_REGIONS + 1];
>>  struct mem_region *mem_regions = __initial_mem_regions;
>>  phys_addr_t __phys_offset, __phys_end;
>>  
>> -u32 dcache_line_size;
>> +unsigned long dcache_line_size;
>>  
>>  int mpidr_to_cpu(uint64_t mpidr)
>>  {
>> diff --git a/lib/arm64/asm/processor.h b/lib/arm64/asm/processor.h
>> index 02665b84cc7e..6ee7c1260b6b 100644
>> --- a/lib/arm64/asm/processor.h
>> +++ b/lib/arm64/asm/processor.h
>> @@ -115,7 +115,7 @@ static inline u64 get_ctr(void)
>>  	return read_sysreg(ctr_el0);
>>  }
>>  
>> -extern u32 dcache_line_size;
>> +extern unsigned long dcache_line_size;
>>  
>>  #endif /* !__ASSEMBLY__ */
>>  #endif /* _ASMARM64_PROCESSOR_H_ */
>>
> 
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> 

Queued all three, thanks.

Paolo


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

* Re: [PATCH kvm-unit-tests 3/3] arm/arm64: Change dcache_line_size to ulong
  2020-10-31 14:32     ` Paolo Bonzini
@ 2020-11-11 15:06       ` Andrew Jones
  0 siblings, 0 replies; 10+ messages in thread
From: Andrew Jones @ 2020-11-11 15:06 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Thomas Huth, kvm

On Sat, Oct 31, 2020 at 03:32:45PM +0100, Paolo Bonzini wrote:
> 
> Queued all three, thanks.
>

Hi Paolo,

Can you please push these?

Thanks,
drew


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

end of thread, other threads:[~2020-11-11 15:07 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-14 19:14 [PATCH kvm-unit-tests 0/3] A few miscellaneous fixes Andrew Jones
2020-10-14 19:14 ` [PATCH kvm-unit-tests 1/3] lib/string: Fix getenv name matching Andrew Jones
2020-10-15  8:19   ` Thomas Huth
2020-10-31 14:31   ` Paolo Bonzini
2020-10-14 19:14 ` [PATCH kvm-unit-tests 2/3] scripts: Save rematch before calling out of for_each_unittest Andrew Jones
2020-10-15  8:22   ` Thomas Huth
2020-10-14 19:14 ` [PATCH kvm-unit-tests 3/3] arm/arm64: Change dcache_line_size to ulong Andrew Jones
2020-10-15  8:30   ` Thomas Huth
2020-10-31 14:32     ` Paolo Bonzini
2020-11-11 15:06       ` Andrew Jones

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).