On Tue, Jul 27, 2010 at 10:49:10PM +0100, Thomas Adam wrote:
> On Tue, Jul 27, 2010 at 12:21:17PM -0400, Chris Siebenmann wrote:
> > There's a small file descriptor leak in CMD_Exec() in fvwm/builtins.c;
> > it opens /dev/null, dup2()s it to standard input, and then never closes
> > the original file descriptor before exec'ing the command.
> >
> > The neurotically correct fix for this (in what I think is the right
> > fvwm coding style) is:
> >
> > --- fvwm/builtins.c 31 Dec 2009 17:44:56 -0000 1.437
> > +++ fvwm/builtins.c 27 Jul 2010 16:17:52 -0000
> > @@ -2459,6 +2459,10 @@
> > fvmm_deinstall_signals();
> > fd = open("/dev/null", O_RDONLY, 0);
> > dup2(fd,STDIN_FILENO);
> > + if (fd != STDIN_FILENO)
> > + {
> > + (void)close(fd);
> > + }
> > if (fvwm_setpgrp() == -1)
> > {
> > fvwm_msg(ERR, "exec_function", "setpgrp failed (%s)",
> >
> > (It seems extremely unlikely that fvwm would ever have stdin closed at
> > this point, so the less neurotic fix is just '(void)close(fd);'.)
>
> You've not read any of the documentation about submitting patches.
> Nevermind, I will modify this and apply it to CVS soon -- I can't/won't
> apply it as-is.
Fixed in CVS.
-- Thomas Adam
--
"It was the cruelest game I've ever played and it's played inside my head."
-- "Hush The Warmth", Gorky's Zygotic Mynci.
