From owner-cvs-all  Fri Dec 21 15:55:47 2001
Delivered-To: cvs-all@freebsd.org
Received: from smtp001pub.verizon.net (smtp001pub.verizon.net [206.46.170.180])
	by hub.freebsd.org (Postfix) with ESMTP
	id CFF0837B41C; Fri, 21 Dec 2001 15:55:36 -0800 (PST)
Received: from bellatlantic.net (pool-151-198-135-131.mad.east.verizon.net [151.198.135.131])
	by smtp001pub.verizon.net  with ESMTP
	; id fBLNtMl22616
	Fri, 21 Dec 2001 17:55:22 -0600 (CST)
Message-ID: <3C23CBE8.D7DDEE80@bellatlantic.net>
Date: Fri, 21 Dec 2001 18:55:20 -0500
From: Sergey Babkin <babkin@bellatlantic.net>
Reply-To: babkin@freebsd.org
X-Mailer: Mozilla 4.7 [en] (X11; U; FreeBSD 4.0-19990626-CURRENT i386)
X-Accept-Language: en, ru
MIME-Version: 1.0
To: Bruce Evans <bde@zeta.org.au>
Cc: Erik Trulsson <ertr1013@student.uu.se>,
	Steve Price <steve@FreeBSD.org>, Andreas Klemm <andreas@FreeBSD.org>,
	cvs-committers@FreeBSD.org, cvs-all@FreeBSD.org
Subject: Re: sh patch Re: cvs commit: ports/print/apsfilter Makefile 
 ports/print/apsfilter/files patch-bin::aps2file  
 ports/print/apsfilter/scriptspre-configure
References: <20011222000107.C4679-100000@gamplex.bde.org>
Content-Type: text/plain; charset=koi8-r
Content-Transfer-Encoding: 7bit
Sender: owner-cvs-all@FreeBSD.ORG
Precedence: bulk
List-ID: <cvs-all.FreeBSD.ORG>
List-Archive: <http://docs.freebsd.org/mail/> (Web Archive)
List-Help: <mailto:majordomo@FreeBSD.ORG?subject=help> (List Instructions)
List-Subscribe: <mailto:majordomo@FreeBSD.ORG?subject=subscribe%20cvs-all>
List-Unsubscribe: <mailto:majordomo@FreeBSD.ORG?subject=unsubscribe%20cvs-all>
X-Loop: FreeBSD.ORG

Bruce Evans wrote:
> 
> On Thu, 20 Dec 2001, Sergey Babkin wrote:
> 
> > I wrote a patch that makes /bin/sh redirection to stdout work
> > (attached, apply in src/bin/sh). Please review it and/or test it
> > and let me know, and I'll commit it to -current.
> 
> > --- redir.c   2001/12/21 02:11:14     1.1
> > +++ redir.c   2001/12/21 02:30:03
> > @@ -145,11 +145,8 @@
> >                       }
> >                       if (!try) {
> >                               sv->renamed[fd] = i;
> > -                             close(fd);
> >                       }
> >                       INTON;
> > -             } else {
> > -                     close(fd);
> >               }
> >               if (fd == 0)
> >                       fd0_redirected++;
> > @@ -186,6 +183,7 @@
> >                       error("cannot open %s: %s", fname, errmsg(errno, E_OPEN));
> >  movefd:
> >               if (f != fd) {
> > +                     close(fd);
> >                       copyfd(f, fd);
> >                       close(f);
> >               }
> 
> This seems to introduce a race by moving the close after the INTON.

That seems to be OK: in popredir() it first closes the old 
descriptor then moves the saved descriptor to the new descriptor,
in clearredir() it just closes the saved descriptors and leaves
the old descriptors alone. My understanding is that INTOFF and
INTON here guard against the waste of the newly opened saved
descriptors: when an old descriptor is dup-ed to the saved one
but this fact is not recorded yet in sv->renamed[fd]. close()
itself does not seem to be directly related to this issue.
In fact, when the old descriptor is not saved, this close:

> > -             } else {
> > -                     close(fd);
> >               }

was not protected by INTOFF and apparently this did not cause any
problems.

-SB

To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe cvs-all" in the body of the message