Bug#290507: vim does not close file upon :sh

[James Vega]

On Wed, Jul 29, 2009 at 02:20:32PM +0200, Bram Moolenaar wrote:
> 
> James -
> 
> > > On Sat, Oct 07, 2006 at 03:32:45PM +0200, Bram Moolenaar wrote:
> > > 
> > > ugh, cough, that's a dusty email!
> > 
> > I finally had a chance to look into this and find how we could avoid it.
> > :)
> > 
> > > On Sat, Oct 07, 2006 at 03:32:45PM +0200, Bram Moolenaar wrote:
> > > > > Hmm, perhaps the problem is that system() or execvp() doesn't close the
> > > > > file descriptors in a child process?
> > > > 
> > > > Indeed, that is the case.  Children inherit their parent's file
> > > > descriptors unless the file descriptors are set to close on exec().
> > > > Attached patch does that.
> > > 
> > > I don't think the FD_CLOEXEC flag is always available.  I'm not sure
> > > when it was added.  Adding an autoconf check for it will help to avoid
> > > breaking the build on old systems.
> > 
> > I don't know autoconf, so I'm afraid I wouldn't be able to provide this
> > part of the patch any time soon.  FD_CLOEXEC is required by POSIX
> > though, so I'm unsure how much this check is really needed.
> > 
> > > This page mentions that one should get the flags first:
> > > http://www.cs.ui.ac.id/WebKuliah/IKI10100/resources/contest/OnlineJudge/gnudoc/libc/Descriptor_Flags.html
> > 
> > Ah, yes.  Currently FD_CLOEXEC is the only supported flag, but that
> > would be the proper way to do this for long-term robustness.  Should I
> > update this part of the patch or will you take care of it with the
> > autoconf checks?
> 
> I would appreciate it if you can update the patch as far as you can and
> test it.  Then I'll have a look at it as well.  It's always better to
> have more than one person look at these things to avoid mistakes.

Updated patch attached.  The flags are retrieved first and F_SETFD is
only called if that was successful and FD_CLOEXEC isn't already set.
I've also added an autoconf check which defines HAVE_FD_CLOEXEC if it is
successful.  That is also used to guard the related code instead of the
previous "#ifdef UNIX".

I followed the model of similar autoconf checks for the FD_CLOEXEC
check.  I think it should be sufficient but I don't have any systems
which have fcntl.h yet don't define FD_CLOEXEC.

-- 
James
GPG Key: 1024D/61326D40 2003-09-02 James Vega <jamessan at debian.org>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: Digital signature
URL: <http://lists.alioth.debian.org/pipermail/pkg-vim-maintainers/attachments/20090811/727f2eec/attachment.pgp>