Closed Bug 719164 Opened 9 years ago Closed 9 years ago

Silence glxtest to get rid of spurious messages from the GL


(Core :: Graphics, defect)

Not set





(Reporter: bjacob, Assigned: bjacob)



(1 file, 1 obsolete file)

Attached patch close file descriptors (obsolete) — Splinter Review
Several users have been confused by and have reported to us 'Failed to create drawable' messages from Mesa. It's also a bit confusing to have the glxtest process dump a XPCOM strings leak log.

closing these file descriptors does cause some 'EBADF' errors but afaics it shouldn't matter.

Also in the same patch, a little cleanup of the code determining the filename of the GL library.
Attachment #589588 - Flags: review?(mh+mozilla)
Comment on attachment 589588 [details] [diff] [review]
close file descriptors

Review of attachment 589588 [details] [diff] [review]:

::: toolkit/xre/glxtest.cpp
@@ +112,5 @@
>  {
> +  // close stdout and stderr to prevent spurious output from the GL from appearing in the terminal.
> +  // in particular, Mesa "Failed to create drawable" messages have repeatedly confused people.
> +  close(STDOUT_FILENO);
> +  close(STDERR_FILENO);

Using something like:
  int fd = open("/dev/null", O_RDONLY);
  dup2(fd, STDOUT_FILENO);
  dup2(fd, STDERR_FILENO);

will avoid the EBADF errors.
Attachment #589588 - Flags: review?(mh+mozilla) → review-
err, O_WRONLY instead of O_RDONLY, obviously.
In fact, you should probably do so for any file descriptor < fd, because XPCOM leaks log may be set to use a file and will pollute there.
The code proposed in comment 1 and comment 2 does remove the EBADF, but printf then generates ENOTTY errors as explained there:

The idea of comment 3 is neat but seems hard to implement in a portable way:

Whatever we do here, being low-maintenance should be the first criterion...
(In reply to Benoit Jacob [:bjacob] from comment #4)
> The code proposed in comment 1 and comment 2 does remove the EBADF, but
> printf then generates ENOTTY errors as explained there:

This doesn't happen here on a small testcase. Does it still happen if you fflush(stdout) and fflush(stderr) before dup2()ing?

> The idea of comment 3 is neat but seems hard to implement in a portable way:
> descriptor

The code doesn't need to be very portable, since it's X11 specific. Moreover, we don't need to care about getting the highest allocated file descriptor. We only care about trying to close the xpcom logging ones, which, in all likeliness, are below "fd" (the value we get from opening /dev/null)
#include <stdio.h>
#include <unistd.h>
#include <errno.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>

int main() {
  int fd = open("/dev/null", O_WRONLY);
  fprintf(stderr, "after open, errno=%d\n", errno);
  dup2(fd, STDOUT_FILENO);
  fprintf(stderr, "after dup2, errno=%d\n", errno);
  fprintf(stderr, "after close, errno=%d\n", errno);
  fprintf(stderr, "after printf, errno=%d\n", errno);


$ g++ a.cpp -o a && ./a
after open, errno=0
after dup2, errno=0
after close, errno=0
after printf, errno=25
Ok, so it wasn't happening on my small test because i was doing a printf before setting stdout to /dev/null, and that doesn't do ENOTTY. Another interesting thing, if just do a main() { printf() } kind of program and run it redirected to /dev/null (./a > /dev/null), it does ENOTTY as well. So it shouldn't matter much.
Is there a reason to believe that EBADF is more of a problem than ENOTTY?

anyway, patch coming.
Attachment #589588 - Attachment is obsolete: true
Attachment #593178 - Flags: review?(mh+mozilla)
Comment on attachment 593178 [details] [diff] [review]
redirect all file descriptors to null

Review of attachment 593178 [details] [diff] [review]:

::: toolkit/xre/glxtest.cpp
@@ +61,5 @@
> +#ifdef LINUX
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#endif

What do you need these for? r+ with these removed.
Attachment #593178 - Flags: review?(mh+mozilla) → review+
They just were mentioned by the open(2) man page, and I seem to remember something like that on Fedora 13 a while ago, but I could be wrong. A version without these includes is now on try:
ah, you may need them for the third argument to open, when creating a file, to use macros instead of hardcoding octal values. for normal uses fnctl.h is enough.

You were right, these includes weren't needed.
Assignee: nobody → bjacob
Target Milestone: --- → mozilla13
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.