test-skeleton.c: Use stdout for error messages

At the moment the test skeleton uses a mixture of stdout and
stderr for error message output. Using stdout for all test output
keeps all output correctly ordered and properly redirected to the
output file. The suggestion to use stdout is also made on the wiki:

https://sourceware.org/glibc/wiki/Testing/Testsuite#Writing_a_test_case

ChangeLog:

2014-04-11  Will Newton  <will.newton@linaro.org>

	* test-skeleton.c (signal_handler): Use printf and strerror
	rather than perror.  Use printf rather than fprintf to
	stderr.
	(main): Likewise.
---
 test-skeleton.c | 44 +++++++++++++++++++++-----------------------
 1 file changed, 21 insertions(+), 23 deletions(-)

On Fri, Apr 11, 2014 at 03:49:29PM +0100, Will Newton wrote:
> At the moment the test skeleton uses a mixture of stdout and
> stderr for error message output. Using stdout for all test output
> keeps all output correctly ordered and properly redirected to the
> output file. The suggestion to use stdout is also made on the wiki:
> 
> https://sourceware.org/glibc/wiki/Testing/Testsuite#Writing_a_test_case
> 
> ChangeLog:
> 
> 2014-04-11  Will Newton  <will.newton@linaro.org>
> 
> 	* test-skeleton.c (signal_handler): Use printf and strerror
> 	rather than perror.  Use printf rather than fprintf to
> 	stderr.
> 	(main): Likewise.
> ---
>  test-skeleton.c | 44 +++++++++++++++++++++-----------------------
>  1 file changed, 21 insertions(+), 23 deletions(-)
> 
> diff --git a/test-skeleton.c b/test-skeleton.c
> index d7d2f75..dd7de8b 100644
> --- a/test-skeleton.c
> +++ b/test-skeleton.c
> @@ -160,7 +160,7 @@ signal_handler (int sig __attribute__ ((unused)))
>      }
>    if (killed != 0 && killed != pid)
>      {
> -      perror ("Failed to kill test process");
> +      printf ("Failed to kill test process: %s\n", strerror (errno));

You could just use %m here.  Likewise for other instances of strerror.

>        exit (1);
>      }
>  
> @@ -181,16 +181,16 @@ signal_handler (int sig __attribute__ ((unused)))
>  #endif
>  
>    if (killed == 0 || (WIFSIGNALED (status) && WTERMSIG (status) == SIGKILL))
> -    fputs ("Timed out: killed the child process\n", stderr);
> +    puts ("Timed out: killed the child process\n");
>    else if (WIFSTOPPED (status))
> -    fprintf (stderr, "Timed out: the child process was %s\n",
> -	     strsignal (WSTOPSIG (status)));
> +    printf ("Timed out: the child process was %s\n",
> +	    strsignal (WSTOPSIG (status)));
>    else if (WIFSIGNALED (status))
> -    fprintf (stderr, "Timed out: the child process got signal %s\n",
> -	     strsignal (WTERMSIG (status)));
> +    printf ("Timed out: the child process got signal %s\n",
> +	    strsignal (WTERMSIG (status)));
>    else
> -    fprintf (stderr, "Timed out: killed the child process but it exited %d\n",
> -	     WEXITSTATUS (status));
> +    printf ("Timed out: killed the child process but it exited %d\n",
> +	    WEXITSTATUS (status));
>  
>    /* Exit with an error.  */
>    exit (1);
> @@ -275,7 +275,7 @@ main (int argc, char *argv[])
>  
>        if (chdir (test_dir) < 0)
>  	{
> -	  perror ("chdir");
> +	  printf ("chdir: %s\n", strerror (errno));
>  	  exit (1);
>  	}
>      }
> @@ -334,10 +334,10 @@ main (int argc, char *argv[])
>  	    data_limit.rlim_cur = MIN ((rlim_t) TEST_DATA_LIMIT,
>  				       data_limit.rlim_max);
>  	  if (setrlimit (RLIMIT_DATA, &data_limit) < 0)
> -	    perror ("setrlimit: RLIMIT_DATA");
> +	    printf ("setrlimit: RLIMIT_DATA: %s\n", strerror (errno));
>  	}
>        else
> -	perror ("getrlimit: RLIMIT_DATA");
> +	printf ("getrlimit: RLIMIT_DATA: %s\n", strerror (errno));
>  #endif
>  
>        /* We put the test process in its own pgrp so that if it bogusly
> @@ -349,7 +349,7 @@ main (int argc, char *argv[])
>      }
>    else if (pid < 0)
>      {
> -      perror ("Cannot fork test program");
> +      printf ("Cannot fork test program: %s\n", strerror (errno));
>        exit (1);
>      }
>  
> @@ -387,18 +387,16 @@ main (int argc, char *argv[])
>        if (EXPECTED_SIGNAL != 0)
>  	{
>  	  if (WTERMSIG (status) == 0)
> -	    fprintf (stderr,
> -		     "Expected signal '%s' from child, got none\n",
> -		     strsignal (EXPECTED_SIGNAL));
> +	    printf ("Expected signal '%s' from child, got none\n",
> +		    strsignal (EXPECTED_SIGNAL));
>  	  else
> -	    fprintf (stderr,
> -		     "Incorrect signal from child: got `%s', need `%s'\n",
> -		     strsignal (WTERMSIG (status)),
> -		     strsignal (EXPECTED_SIGNAL));
> +	    printf ("Incorrect signal from child: got `%s', need `%s'\n",
> +		    strsignal (WTERMSIG (status)),
> +		    strsignal (EXPECTED_SIGNAL));
>  	}
>        else
> -	fprintf (stderr, "Didn't expect signal from child: got `%s'\n",
> -		 strsignal (WTERMSIG (status)));
> +	printf ("Didn't expect signal from child: got `%s'\n",
> +		strsignal (WTERMSIG (status)));
>        exit (1);
>      }
>  
> @@ -408,8 +406,8 @@ main (int argc, char *argv[])
>  #else
>    if (WEXITSTATUS (status) != EXPECTED_STATUS)
>      {
> -      fprintf (stderr, "Expected status %d, got %d\n",
> -	       EXPECTED_STATUS, WEXITSTATUS (status));
> +      printf ("Expected status %d, got %d\n",
> +	      EXPECTED_STATUS, WEXITSTATUS (status));
>        exit (1);
>      }
>  

This looks good to me with the above changes, but I think a senior
maintainer should review this as well before it goes in.

Siddhesh

On Fri, 11 Apr 2014, Siddhesh Poyarekar wrote:

> > diff --git a/test-skeleton.c b/test-skeleton.c
> > index d7d2f75..dd7de8b 100644
> > --- a/test-skeleton.c
> > +++ b/test-skeleton.c
> > @@ -160,7 +160,7 @@ signal_handler (int sig __attribute__ ((unused)))
> >      }
> >    if (killed != 0 && killed != pid)
> >      {
> > -      perror ("Failed to kill test process");
> > +      printf ("Failed to kill test process: %s\n", strerror (errno));
> 
> You could just use %m here.  Likewise for other instances of strerror.

Agreed.

> >    if (killed == 0 || (WIFSIGNALED (status) && WTERMSIG (status) == SIGKILL))
> > -    fputs ("Timed out: killed the child process\n", stderr);
> > +    puts ("Timed out: killed the child process\n");

puts outputs a newline implicitly, so when converting to puts you should 
remove the \n from the string.

> This looks good to me with the above changes, but I think a senior
> maintainer should review this as well before it goes in.

The patch is OK with the requested changes.

On 19 June 2014 22:49, Joseph S. Myers <joseph@codesourcery.com> wrote:
> On Fri, 11 Apr 2014, Siddhesh Poyarekar wrote:
>
>> > diff --git a/test-skeleton.c b/test-skeleton.c
>> > index d7d2f75..dd7de8b 100644
>> > --- a/test-skeleton.c
>> > +++ b/test-skeleton.c
>> > @@ -160,7 +160,7 @@ signal_handler (int sig __attribute__ ((unused)))
>> >      }
>> >    if (killed != 0 && killed != pid)
>> >      {
>> > -      perror ("Failed to kill test process");
>> > +      printf ("Failed to kill test process: %s\n", strerror (errno));
>>
>> You could just use %m here.  Likewise for other instances of strerror.
>
> Agreed.
>
>> >    if (killed == 0 || (WIFSIGNALED (status) && WTERMSIG (status) == SIGKILL))
>> > -    fputs ("Timed out: killed the child process\n", stderr);
>> > +    puts ("Timed out: killed the child process\n");
>
> puts outputs a newline implicitly, so when converting to puts you should
> remove the \n from the string.
>
>> This looks good to me with the above changes, but I think a senior
>> maintainer should review this as well before it goes in.
>
> The patch is OK with the requested changes.

Thanks, pushed with those changes.