[PATCH v3] tests for fenv.h functions

[Eshan Dhawan]

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.
>
>> > +#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
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/devel/attachments/20200305/71f805c0/attachment.html>