[PATCH v3] tests for fenv.h functions

[Gedare Bloom]

On Thu, Mar 5, 2020 at 12:17 AM Eshan Dhawan <eshandhawan51 at gmail.com> wrote:
>
> If you want I would set the case in that manner.
>
> On Thu, Mar 5, 2020 at 12:46 PM Eshan Dhawan <eshandhawan51 at gmail.com> wrote:
>>
>>
>>
>> On Thu, Mar 5, 2020 at 4:37 AM Gedare Bloom <gedare at rtems.org> wrote:
>>>
>>> On Wed, Mar 4, 2020 at 11:12 AM Eshan dhawan <eshandhawan51 at gmail.com> wrote:
>>> >
>>> > ---
>>> >  testsuites/psxtests/psxfenv01/init.c | 72 ++++++++++++++++++++++++++--
>>> >  1 file changed, 68 insertions(+), 4 deletions(-)
>>> >
>>> > diff --git a/testsuites/psxtests/psxfenv01/init.c b/testsuites/psxtests/psxfenv01/init.c
>>> > index cdb0fa596e..40c29e4fac 100644
>>> > --- a/testsuites/psxtests/psxfenv01/init.c
>>> > +++ b/testsuites/psxtests/psxfenv01/init.c
>>> > @@ -46,6 +46,10 @@
>>> >  #include <string.h>
>>> >  #include <rtems/test.h>
>>> >  #include <tmacros.h>
>>> > +#include <assert.h>
>>> I don't think this include is necessary?
>>>
>>> > +#include <float.h>
>>> > +
>>> > +
>>> Don't add extra vertical space unnecessarily. In general, there should
>>> never be more than 1 blank line consecutively anywhere.
>>> >
>>> >  const char rtems_test_name[] = "PSXFENV 01";
>>> >
>>> > @@ -69,20 +73,23 @@ rtems_task Init(rtems_task_argument ignored)
>>> >    #ifdef FE_ALL_EXCEPT /* floating-point exceptions */
>>> >      puts( "fesetenv(FE_DFL_ENV)." );
>>> >      r = fesetenv(FE_DFL_ENV);
>>> > -    if (r)
>>> > +    if (r){
>>> Almost, need some more spaces around 'r' and between '){', like this:
>>> "if ( r ) {"
>>>
>>> >        printf("fesetenv ==> %d\n", r);
>>> > +    }
>>> >      rtems_test_assert( r == 0 );
>>> >
>>> >      /* Test 'feclearexcept()' and 'fetestexcept()' in one go. */
>>> >      puts( "feclearexcept(FE_ALL_EXCEPT)." );
>>> >      r = feclearexcept(FE_ALL_EXCEPT);
>>> > -    if (r)
>>> > +    if (r){
>>> >        printf("feclearexcept ==> 0x%x\n", r);
>>> > +    }
>>> >      rtems_test_assert( r == 0 );
>>> >
>>> >      r = fetestexcept( FE_ALL_EXCEPT );
>>> > -    if (r)
>>> > +    if (r){
>>> >        printf("fetestexcept ==> 0x%x\n", r);
>>> > +    }
>>> >      rtems_test_assert( r == 0 );
>>> >
>>> >      /* Test 'FE_DIVBYZERO' */
>>> > @@ -91,8 +98,63 @@ rtems_task Init(rtems_task_argument ignored)
>>> >      b = 1.0;
>>> >      c = b/a;
>>> >      (void) c;
>>> > +    /* Test 'fegetexceptflag()' and 'fesetexceptflag()' */
>>> > +    r = fegetexceptflag(&excepts,FE_ALL_EXCEPT);
>>> also need to add whitespace after commas.
>>>
>>> > +    if(r){
>>> > +      printf("fegetexceptflag ==> 0x%x\n", r);
>>> > +    }
>>> > +    rtems_test_assert(r == 0);
>>> > +
>>> > +    r = fesetexceptflag(&excepts, FE_ALL_EXCEPT);
>>> spaces between ( ).
>>> > +    if(r){
>>> > +      printf("fesetexceptflag ==> 0x%x\n", r);
>>> > +    }
>>> > +    rtems_test_assert(r == 0);
>>> ditto
>>>
>>> > +
>>> > +
>>> 2 blanks
>>>
>>> > +    /* Test for 'fegetround()' and 'fesetround()' */
>>> > +    /* They have four main macros to be tested seperated by ifdef*/
>>> > +    /* Since not all architectures support them */
>>> > +    /* The test case gets and sets the rounding directions */
>>> Merge multiline comments with a single comment block
>>> /* Test for ...
>>>  * They have ...
>>>  * Since not ...
>>>  * The test case ...
>>>  */
>>>
>>> > +#ifdef FE_TONEAREST
>>> > +    r = fegetround();
>>> > +    if(r){
>>> > +      printf("fegetround ==> 0x%x\n", r);
>>> > +    }
>>> > +    rtems_test_assert(r == FE_TONEAREST) ;
>>> Is FE_TONEAREST expected to be the default behavior without any calls
>>> to fesetround?
>>
>> No, We have to define the rounding direction flag in the function call.
>> FE_TONEAREST is the default rounding direction if nothing is specified.
>>>
>>> Why not do something like you did below, with rtems_test_assert(
>>> fegetround() == FE_TONEAREST );?
>>
>> I have used this way only in this test case because if there is an error with fegetround(),
>> we could get the error code.
>>>
>>> > +#endif
>>> > +#ifdef FE_TOWARDZERO
>>> > +  r = fesetround(FE_TOWARDZERO);
>>> > +  if(r){
>>> > +      printf("fesetround ==> 0x%x\n", r);
>>> > +    }
>>> > +  rtems_test_assert(r == 0) ;
>>> > +  rtems_test_assert(fegetround() == FE_TOWARDZERO) ;
>>> This is a nice, concise way to test the rounding condition. Why not do
>>> the same above with FE_TONEAREST?
>>
>>  we wouldn't get any error code sent by the function in this method.
>> So, if there is an error in the default then I would want it to be displayed.

rtems_test_assert macro will print the condition that failed, so the
display would be redundant. You can just use the simpler, concise
version to make a smaller code.

>>>
>>> > +#endif
>>> > +#ifdef FE_DOWNWARD
>>> > +  r = fesetround(FE_DOWNWARD);
>>> > +  if(r){
>>> > +      printf("fesetround ==> 0x%x\n", r);
>>> > +    }
>>> > +  rtems_test_assert(r == 0) ;
>>> > +  rtems_test_assert(fegetround() == FE_DOWNWARD) ;
>>> > +#endif
>>> > +#ifdef FE_UPWARD
>>> > +  r = fesetround(FE_UPWARD);
>>> > +  if(r){
>>> > +      printf("fesetround ==> 0x%x\n", r);
>>> > +    }
>>> > +  rtems_test_assert(r == 0) ;
>>> > +  rtems_test_assert(fegetround() == FE_UPWARD) ;
>>> > +#endif
>>> > +#ifdef FE_TONEAREST
>>> > +  r = fesetround(FE_TONEAREST);
>>> > +  if(r){
>>> > +      printf("fesetround ==> 0x%x\n", r);
>>> > +    }
>>> > +  rtems_test_assert(r == 0) ;
>>> > +#endif
>>> >
>>> > -    fegetexceptflag(&excepts,FE_ALL_EXCEPT);
>>> >
>>> >  #ifdef FE_DIVBYZERO
>>> >      r = feraiseexcept(FE_DIVBYZERO);
>>> > @@ -125,3 +187,5 @@ rtems_task Init(rtems_task_argument ignored)
>>> >  #define CONFIGURE_INIT
>>> >  #include <rtems/confdefs.h>
>>> >  /* end of file */
>>> > +
>>> > +
>>> Don't add blank lines randomly.
>>>
>>> > --
>>> > 2.17.1
>>> >
>>> > _______________________________________________
>>> > devel mailing list
>>> > devel at rtems.org
>>> > http://lists.rtems.org/mailman/listinfo/devel