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