* [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
* 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 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
* [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
* 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
* [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 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 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