diff mbox series

kselftest: vDSO: Fix accumulation of uninitialized ret when CLOCK_REALTIME is undefined

Message ID 20230417104743.30018-1-colin.i.king@gmail.com
State Accepted
Commit 375b9ff53cb6f9c042817b75f2be0a650626dc4f
Headers show
Series kselftest: vDSO: Fix accumulation of uninitialized ret when CLOCK_REALTIME is undefined | expand

Commit Message

Colin Ian King April 17, 2023, 10:47 a.m. UTC
In the unlikely case that CLOCK_REALTIME is not defined, variable ret is
not initialized and further accumulation of return values to ret can leave
ret in an undefined state. Fix this by initialized ret to zero and changing
the assignment of ret to an accumulation for the CLOCK_REALTIME case.

Fixes: 03f55c7952c9 ("kselftest: Extend vDSO selftest to clock_getres")
Signed-off-by: Colin Ian King <colin.i.king@gmail.com>
---
 tools/testing/selftests/vDSO/vdso_test_clock_getres.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Vincenzo Frascino April 18, 2023, 10:10 a.m. UTC | #1
Hi Colin,

On 4/17/23 11:47, Colin Ian King wrote:
> In the unlikely case that CLOCK_REALTIME is not defined, variable ret is
> not initialized and further accumulation of return values to ret can leave
> ret in an undefined state. Fix this by initialized ret to zero and changing
> the assignment of ret to an accumulation for the CLOCK_REALTIME case.
> 

I was wondering how did you find this.

Apart that:

Reviewed-by: Vincenzo Frascino <vincenzo.frascino@arm.com>

> Fixes: 03f55c7952c9 ("kselftest: Extend vDSO selftest to clock_getres")
> Signed-off-by: Colin Ian King <colin.i.king@gmail.com>
> ---
>  tools/testing/selftests/vDSO/vdso_test_clock_getres.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/vDSO/vdso_test_clock_getres.c b/tools/testing/selftests/vDSO/vdso_test_clock_getres.c
> index 15dcee16ff72..38d46a8bf7cb 100644
> --- a/tools/testing/selftests/vDSO/vdso_test_clock_getres.c
> +++ b/tools/testing/selftests/vDSO/vdso_test_clock_getres.c
> @@ -84,12 +84,12 @@ static inline int vdso_test_clock(unsigned int clock_id)
>  
>  int main(int argc, char **argv)
>  {
> -	int ret;
> +	int ret = 0;
>  
>  #if _POSIX_TIMERS > 0
>  
>  #ifdef CLOCK_REALTIME
> -	ret = vdso_test_clock(CLOCK_REALTIME);
> +	ret += vdso_test_clock(CLOCK_REALTIME);
>  #endif
>  
>  #ifdef CLOCK_BOOTTIME
Colin Ian King April 18, 2023, 10:14 a.m. UTC | #2
On 18/04/2023 11:10, Vincenzo Frascino wrote:
> Hi Colin,
> 
> On 4/17/23 11:47, Colin Ian King wrote:
>> In the unlikely case that CLOCK_REALTIME is not defined, variable ret is
>> not initialized and further accumulation of return values to ret can leave
>> ret in an undefined state. Fix this by initialized ret to zero and changing
>> the assignment of ret to an accumulation for the CLOCK_REALTIME case.
>>
> 
> I was wondering how did you find this.

I used cppcheck --force --enable=all, this examines the #if defined() paths.

> 
> Apart that:
> 
> Reviewed-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
> 
>> Fixes: 03f55c7952c9 ("kselftest: Extend vDSO selftest to clock_getres")
>> Signed-off-by: Colin Ian King <colin.i.king@gmail.com>
>> ---
>>   tools/testing/selftests/vDSO/vdso_test_clock_getres.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/testing/selftests/vDSO/vdso_test_clock_getres.c b/tools/testing/selftests/vDSO/vdso_test_clock_getres.c
>> index 15dcee16ff72..38d46a8bf7cb 100644
>> --- a/tools/testing/selftests/vDSO/vdso_test_clock_getres.c
>> +++ b/tools/testing/selftests/vDSO/vdso_test_clock_getres.c
>> @@ -84,12 +84,12 @@ static inline int vdso_test_clock(unsigned int clock_id)
>>   
>>   int main(int argc, char **argv)
>>   {
>> -	int ret;
>> +	int ret = 0;
>>   
>>   #if _POSIX_TIMERS > 0
>>   
>>   #ifdef CLOCK_REALTIME
>> -	ret = vdso_test_clock(CLOCK_REALTIME);
>> +	ret += vdso_test_clock(CLOCK_REALTIME);
>>   #endif
>>   
>>   #ifdef CLOCK_BOOTTIME
>
Shuah Khan May 8, 2023, 5:37 p.m. UTC | #3
On 4/18/23 04:14, Colin King (gmail) wrote:
> On 18/04/2023 11:10, Vincenzo Frascino wrote:
>> Hi Colin,
>>
>> On 4/17/23 11:47, Colin Ian King wrote:
>>> In the unlikely case that CLOCK_REALTIME is not defined, variable ret is
>>> not initialized and further accumulation of return values to ret can leave
>>> ret in an undefined state. Fix this by initialized ret to zero and changing
>>> the assignment of ret to an accumulation for the CLOCK_REALTIME case.
>>>
>>
>> I was wondering how did you find this.
> 
> I used cppcheck --force --enable=all, this examines the #if defined() paths.
> 
>>
>> Apart that:
>>
>> Reviewed-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
>>

Applied to linux-kselftest next

thanks,
-- Shuah
diff mbox series

Patch

diff --git a/tools/testing/selftests/vDSO/vdso_test_clock_getres.c b/tools/testing/selftests/vDSO/vdso_test_clock_getres.c
index 15dcee16ff72..38d46a8bf7cb 100644
--- a/tools/testing/selftests/vDSO/vdso_test_clock_getres.c
+++ b/tools/testing/selftests/vDSO/vdso_test_clock_getres.c
@@ -84,12 +84,12 @@  static inline int vdso_test_clock(unsigned int clock_id)
 
 int main(int argc, char **argv)
 {
-	int ret;
+	int ret = 0;
 
 #if _POSIX_TIMERS > 0
 
 #ifdef CLOCK_REALTIME
-	ret = vdso_test_clock(CLOCK_REALTIME);
+	ret += vdso_test_clock(CLOCK_REALTIME);
 #endif
 
 #ifdef CLOCK_BOOTTIME