[v2] test-skeleton: redirect stderr to stdout

Message ID 1466141361-4765-1-git-send-email-vapier@gentoo.org
State New, archived
Headers

Commit Message

Mike Frysinger June 17, 2016, 5:29 a.m. UTC
  Rather than worry if we use funcs that dirty stderr instead of writing
to stdout, redirect stderr internally to stdout.  Now all output will
go to stdout regardless.
---
 test-skeleton.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)
  

Comments

Pedro Alves June 17, 2016, 11:12 a.m. UTC | #1
On 06/17/2016 06:29 AM, Mike Frysinger wrote:

> +  /* Bind stderr to stdout so that tests don't have to worry about which
> +     one to use, and whether funcs they use (e.g. assert) will go to the
> +     wrong place.  */
> +  fclose (stderr);
> +  if (dup2 (STDOUT_FILENO, STDERR_FILENO) == -1)
> +    {
> +      printf ("binding stderr to stdout failed: %m\n");
> +      exit (1);
> +    }
> +  stderr = fdopen (STDERR_FILENO, "w");
> +  /* Since stderr always starts out unbuffered, recreate that here.  */
> +  setbuf (stderr, NULL);

Why do you need to fclose/fdopen stderr at all?
Just the "dup2 (STDOUT_FILENO, STDERR_FILENO)" should be enough.

Note that that's what

 http://pubs.opengroup.org/onlinepubs/9699919799/functions/dup.html

suggests in its example section:

~~~~~~~~~~~~~~~~~~~~~
 Redirecting Error Messages

 The following example redirects messages from stderr to stdout.

 #include <unistd.h>
 ...
 dup2(1, 2);
 ...
~~~~~~~~~~~~~~~~~~~~~

Thanks,
Pedro Alves
  
Florian Weimer June 17, 2016, 11:23 a.m. UTC | #2
On 06/17/2016 07:29 AM, Mike Frysinger wrote:
> +  stderr = fdopen (STDERR_FILENO, "w");

Currently, test-skeleton.c does not cause malloc to be called before 
TEST_FUNCTION is invoked (atexit uses a preallocated control block). 
The above changes that, which has some impact on current and future 
malloc-related tests.

Florian
  
Mike Frysinger June 17, 2016, 2:47 p.m. UTC | #3
On 17 Jun 2016 12:12, Pedro Alves wrote:
> On 06/17/2016 06:29 AM, Mike Frysinger wrote:
> 
> > +  /* Bind stderr to stdout so that tests don't have to worry about which
> > +     one to use, and whether funcs they use (e.g. assert) will go to the
> > +     wrong place.  */
> > +  fclose (stderr);
> > +  if (dup2 (STDOUT_FILENO, STDERR_FILENO) == -1)
> > +    {
> > +      printf ("binding stderr to stdout failed: %m\n");
> > +      exit (1);
> > +    }
> > +  stderr = fdopen (STDERR_FILENO, "w");
> > +  /* Since stderr always starts out unbuffered, recreate that here.  */
> > +  setbuf (stderr, NULL);
> 
> Why do you need to fclose/fdopen stderr at all?
> Just the "dup2 (STDOUT_FILENO, STDERR_FILENO)" should be enough.

we discussed it here where i asked for docs:
https://sourceware.org/ml/libc-alpha/2016-06/msg00636.html

> Note that that's what
> 
>  http://pubs.opengroup.org/onlinepubs/9699919799/functions/dup.html
> 
> suggests in its example section:
> 
> ~~~~~~~~~~~~~~~~~~~~~
>  Redirecting Error Messages
> 
>  The following example redirects messages from stderr to stdout.
> 
>  #include <unistd.h>
>  ...
>  dup2(1, 2);
>  ...
> ~~~~~~~~~~~~~~~~~~~~~

that only guarantees the fd level.  where do we guarantee the stdio level ?
-mike
  
Arjun Shankar June 17, 2016, 3:34 p.m. UTC | #4
> Rather than worry if we use funcs that dirty stderr instead of writing
> to stdout, redirect stderr internally to stdout.  Now all output will
> go to stdout regardless.

> +  if (dup2 (STDOUT_FILENO, STDERR_FILENO) == -1)
> +    {
> +      printf ("binding stderr to stdout failed: %m\n");
> +      exit (1);
> +    }

Just want to mention this somewhat old but related thread:

https://sourceware.org/ml/libc-alpha/2014-10/msg00089.html

In short:
While I am not aware of any test that _requires_ writes to stderr, this
blanket `dup2' in the skeleton can affect such tests. Roland mentioned that
for such tests, if they show up, this behaviour can be made to depend on a
macro. I don't know if including a `#ifdef' in this first patch is good
since it probably will remain unused for now, but it's an idea.

Cheers,
Arjun
  
Pedro Alves June 17, 2016, 3:34 p.m. UTC | #5
On 06/17/2016 03:47 PM, Mike Frysinger wrote:
> On 17 Jun 2016 12:12, Pedro Alves wrote:
>> On 06/17/2016 06:29 AM, Mike Frysinger wrote:
>>
>>> +  /* Bind stderr to stdout so that tests don't have to worry about which
>>> +     one to use, and whether funcs they use (e.g. assert) will go to the
>>> +     wrong place.  */
>>> +  fclose (stderr);
>>> +  if (dup2 (STDOUT_FILENO, STDERR_FILENO) == -1)
>>> +    {
>>> +      printf ("binding stderr to stdout failed: %m\n");
>>> +      exit (1);
>>> +    }
>>> +  stderr = fdopen (STDERR_FILENO, "w");
>>> +  /* Since stderr always starts out unbuffered, recreate that here.  */
>>> +  setbuf (stderr, NULL);
>>
>> Why do you need to fclose/fdopen stderr at all?
>> Just the "dup2 (STDOUT_FILENO, STDERR_FILENO)" should be enough.
> 
> we discussed it here where i asked for docs:
> https://sourceware.org/ml/libc-alpha/2016-06/msg00636.html

I don't have a reference handy.  However, this is how everyone
does it.  If it doesn't work, then a ton of programs (and shells) out
there are broken, including all that do:

dup2(..., 0);
dup2(..., 1);
dup2(..., 2);
execvp (...);
perror ("exec failed"); // writes to stderr.
exit(1);


Even the glibc manual shows perror used like that:

 https://www.gnu.org/software/libc/manual/html_node/Launching-Jobs.html#Launching-Jobs


If you may have output pending in stdio's buffers, then call
fflush before the dup2.  But then again stderr is unbuffered
by default, and, if you do already have output pending,
then you're doing this redirection too late, anyway.

> 
>> Note that that's what
>>
>>  http://pubs.opengroup.org/onlinepubs/9699919799/functions/dup.html
>>
>> suggests in its example section:
>>
>> ~~~~~~~~~~~~~~~~~~~~~
>>  Redirecting Error Messages
>>
>>  The following example redirects messages from stderr to stdout.
>>
>>  #include <unistd.h>
>>  ...
>>  dup2(1, 2);
>>  ...
>> ~~~~~~~~~~~~~~~~~~~~~
> 
> that only guarantees the fd level.  where do we guarantee the stdio level ?

The mention of "stderr to stdout" is clearly talking about the
stdio objects.  If to redirect "stderr to stdout" you would need
some further manipulation at the stdio level, then it seems obvious to me
that that example would show what else to do to the stdout/stderr objects.

Thanks,
Pedro Alves
  

Patch

diff --git a/test-skeleton.c b/test-skeleton.c
index a9ad4ab7e9c6..d07d0d76ca11 100644
--- a/test-skeleton.c
+++ b/test-skeleton.c
@@ -343,6 +343,19 @@  main (int argc, char *argv[])
   setbuf (stdout, NULL);
 #endif
 
+  /* Bind stderr to stdout so that tests don't have to worry about which
+     one to use, and whether funcs they use (e.g. assert) will go to the
+     wrong place.  */
+  fclose (stderr);
+  if (dup2 (STDOUT_FILENO, STDERR_FILENO) == -1)
+    {
+      printf ("binding stderr to stdout failed: %m\n");
+      exit (1);
+    }
+  stderr = fdopen (STDERR_FILENO, "w");
+  /* Since stderr always starts out unbuffered, recreate that here.  */
+  setbuf (stderr, NULL);
+
   while ((opt = getopt_long (argc, argv, "+", options, NULL)) != -1)
     switch (opt)
       {