Created attachment 125777 [details] [review]
dbus-sysdeps-unix: Fix a leak of /dev/null on dup2() error path

If this path is taken, devnull has been set to an FD > i, but this value
might be ≤ STDERR_FILENO, in which case the close() call on out will not
be made.

Fix that by calling close() if devnull is higher than the most recent FD
we were looking at in any case.

(In reply to Philip Withnall from comment #1)
> If this path is taken, devnull has been set to an FD > i, but this value
> might be ≤ STDERR_FILENO, in which case the close() call on out will not
> be made.
> 
> Fix that by calling close() if devnull is higher than the most recent FD
> we were looking at in any case.

We should never close(fd) where fd <= STDERR_FILENO: that would bring us back to the bad situation for which this is a workaround, where one of the three standard fds is not open on entry to main(). (Correct parent processes will not do this, but the existence of the X11 autolaunch transport means we cannot rely on our parent being any particular process, and in particular we cannot rely on it being correct.)

So, yes, arguably this is a leak, but it's a desired leak - to the best of our ability, we want fds 0, 1 and 2 to be open. In practice, if _dbus_ensure_standard_fds() is failing we're probably about to crash out anyway, but even so it would seem perverse to move further away from the result it was aiming for.

I've marked the Coverity report as Intentional.

(In reply to Simon McVittie from comment #2)

> I've marked the Coverity report as Intentional.

BTW: I'm performing coverity scans on master branch sporadically on a local build host, which may be suboptimal. Because dbus already uses travis-ci it would be possible to trigger a better timed coverity scan from time to time through travis-ci by simply pushing to 'coverity_scan' branch of github d-bus/dbus git repo. A related travis-ci config example could be inspected at https://github.com/openscenegraph/OpenSceneGraph/blob/master/.travis.yml.

(In reply to Ralf Habacker from comment #3)
> BTW: I'm performing coverity scans on master branch sporadically on a local
> build host, which may be suboptimal. Because dbus already uses travis-ci it
> would be possible to trigger a better timed coverity scan from time to time
> through travis-ci by simply pushing to 'coverity_scan' branch of github
> d-bus/dbus git repo. A related travis-ci config example could be inspected
> at https://github.com/openscenegraph/OpenSceneGraph/blob/master/.travis.yml.

Thanks, that's useful information. I'll look into that when I have some time for QA improvements.