[lxc-devel] [PATCH v3 4/4] uniformly nullify std fds

[Robert Vogelgesang]

Hi,

On Wed, Jun 10, 2015 at 12:04:24PM +0000, Serge Hallyn wrote:
> Quoting Robert Vogelgesang (vogel at users.sourceforge.net):
[...]
> > > +		 * Here, we're taking advantage of C's short circuiting of
> > > +		 * conditions: we should only fail if quiet is set and
> > > +		 * null_stdfds fails.
> > > +		 */
> > > +		if (quiet && null_stdfds() < 0) {
> > > +			exit(1);
> > 
> > My concern is not about someone not understanding short circuiting.
> > There should be a warning against reversing the order of the two parts.
> > 
> > Short circuiting is rather common, but that "quiet" means to close
> > fds, is unusual and not obvious.
> > 
> > If someone would change that to
> > 
> > > +		if (null_stdfds() < 0 && quiet) {
> 
> Then we would hopefully reject the patch.

Once bitten, twice shy:  In a different project (I won't name it here)
I did trust maintainers that they won't do stupid things, with the
consequence of lost data and a major outage, just because the
maintainers did a "harmless" change to silence SOME messages, but
in fact they killed logging of ALL messages of subprocesses.  All
early warnings about upcoming problems were lost.  Since then,
I'm checking each update of this software if they did it again...

A single line comment would make the intention obvious to any reader:

	/* implement "quiet" by redirecting fds 0, 1, 2 to /dev/null */
	if (quiet && null_stdfds() < 0) {

Nobody with at least half a brain would then try to "clean up things"
and reverse the order.

	Robert

> 
> thanks,
> -serge
> _______________________________________________
> lxc-devel mailing list
> lxc-devel at lists.linuxcontainers.org
> http://lists.linuxcontainers.org/listinfo/lxc-devel