[LTP] [PATCH v1] Correctly check setitimer params in setitimer01

[Andrea Cervesato]

Hi,

On 11/2/22 16:39, Martin Doucha wrote:
> Hi,
> one small nit below, otherwise:
>
> Reviewed-by: Martin Doucha <mdoucha@suse.cz>
>
> On 02. 11. 22 15:59, Andrea Cervesato via ltp wrote:
>> Last test rewrite didn't consider the right expected boundaries when
>> setitimer syscall was tested. We also introduced counter times as
>> multiple of clock resolution, to avoid kernel rounding during setitimer
>> counter increase.
>>
>> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
>> ---
>>   .../kernel/syscalls/setitimer/setitimer01.c   | 42 ++++++++++++++-----
>>   1 file changed, 32 insertions(+), 10 deletions(-)
>>
>> diff --git a/testcases/kernel/syscalls/setitimer/setitimer01.c 
>> b/testcases/kernel/syscalls/setitimer/setitimer01.c
>> index f04cb5a69..3fb9250e2 100644
>> --- a/testcases/kernel/syscalls/setitimer/setitimer01.c
>> +++ b/testcases/kernel/syscalls/setitimer/setitimer01.c
>> @@ -8,20 +8,21 @@
>>   /*\
>>    * [Description]
>>    *
>> - * Check that a setitimer() call pass with timer seting.
>> - * Check if signal is generated correctly when timer expiration.
>> + * Spaw a child and verify that setitimer() syscall passes and it 
>> ends up
>> + * counting inside expected boundaries. Then verify from parent that 
>> our syscall
>> + * sent the correct signal to the child.
>>    */
>>   +#include <time.h>
>>   #include <errno.h>
>>   #include <sys/time.h>
>>   #include <stdlib.h>
>>   #include "tst_test.h"
>>   #include "lapi/syscalls.h"
>> -
>> -#define USEC1    10000
>> -#define USEC2    20000
>> +#include "tst_safe_clocks.h"
>>     static struct itimerval *value, *ovalue;
>> +static unsigned long time_step;
>>     static struct tcase {
>>       int which;
>> @@ -55,6 +56,7 @@ static void verify_setitimer(unsigned int i)
>>   {
>>       pid_t pid;
>>       int status;
>> +    int usec = 3 * time_step;
>>       struct tcase *tc = &tcases[i];
>>         pid = SAFE_FORK();
>> @@ -64,14 +66,18 @@ static void verify_setitimer(unsigned int i)
>>             tst_no_corefile(0);
>>   -        set_setitimer_value(USEC1, 0);
>> -        TST_EXP_PASS(sys_setitimer(tc->which, value, NULL));
>> +        set_setitimer_value(usec, 0);
>> +        TST_EXP_PASS(sys_setitimer(tc->which, value, 0));
>
> Why change the third argument from NULL to 0?
>
It's a mistake. I sent patch with 0 because I was tweaking with values 
during tests.
>>   -        set_setitimer_value(USEC2, USEC2);
>> +        set_setitimer_value(5 * time_step, 7 * time_step);
>>           TST_EXP_PASS(sys_setitimer(tc->which, value, ovalue));
>>   -        if (ovalue->it_value.tv_sec != 0 || 
>> ovalue->it_value.tv_usec >= USEC2)
>> -            tst_brk(TFAIL, "old timer value is not within the 
>> expected range");
>> +        tst_res(TINFO, "tv_sec=%ld, tv_usec=%ld",
>> +            ovalue->it_value.tv_sec,
>> +            ovalue->it_value.tv_usec);
>> +
>> +        if (ovalue->it_value.tv_sec != 0 || ovalue->it_value.tv_usec 
>> > usec)
>> +            tst_res(TFAIL, "Ending counters are out of range");
>>             for (;;)
>>               ;
>> @@ -85,10 +91,26 @@ static void verify_setitimer(unsigned int i)
>>           tst_res(TFAIL, "Child: %s", tst_strstatus(status));
>>   }
>>   +static void setup(void)
>> +{
>> +    struct timespec res;
>> +
>> +    SAFE_CLOCK_GETRES(CLOCK_MONOTONIC, &res);
>> +
>> +    time_step = res.tv_nsec / 1000;
>> +    if (time_step < 10000)
>> +        time_step = 10000;
>> +
>> +    tst_res(TINFO, "clock resolution: %luns, time step: %luus",
>> +        res.tv_nsec,
>> +        time_step);
>> +}
>> +
>>   static struct tst_test test = {
>>       .tcnt = ARRAY_SIZE(tcases),
>>       .forks_child = 1,
>>       .test = verify_setitimer,
>> +    .setup = setup,
>>       .bufs = (struct tst_buffers[]) {
>>           {&value,  .size = sizeof(struct itimerval)},
>>           {&ovalue, .size = sizeof(struct itimerval)},
>
Andrea