Re: fd used for saving redirected fds leaked to child processes

[Peter Stephenson <p.stephenson@samsung.com>]

On Sun, 13 Aug 2017 14:45:12 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:
> On Sun, Aug 13, 2017 at 11:49 AM, Peter Stephenson
> <p.w.stephenson@ntlworld.com> wrote:
> >
> > +/* FDs saved for possible restoring, not needed in a subshell
> > + * where we will never need to restore them.  Hence if we enter
> > + * a subshell these will simply be closed unconditionally.
> > + *
> > + * A value >= 10 indicates a valid saved fd.
> > + */
> 
> No objection to defensive programming, but would it not be a bug for
> an fd < 10 to ever be assigned to a slot in saved_fds[] in the first
> place?

Yes, it would --- I just need to exclude the case of 0 as that's the
default (wasn't worth statically initialising to -1's), but checked
the range instead.

> It feels as though there's probably a reason that (int *save) is
> passed around as a parameter to addfd() et al. rather than being a
> global to begin with.  Are you sure there aren't circumstances where
> the same fd might get saved more than once at different levels of the
> recursive execsomething() hierarchy, causing saved_fds[] to contain
> incomplete information?  I'm envisioning something like
> 
> { { cmd1 ; cmd2 } > file2; cmd3 } > file1 &

Yes, exactly that did occur to me later.

We need to expose the entire hierarchy for this particular case, but
that looks like a stack or a linked list, which seems a little
heavyweight for this case.  It needs a bit more thought.  A linked
list rooted at each fd 0 to 9 is the best I've come up with so far.

pws