From owner-freebsd-emulation@FreeBSD.ORG Wed Nov 8 15:07:51 2006 Return-Path: X-Original-To: freebsd-emulation@FreeBSD.org Delivered-To: freebsd-emulation@FreeBSD.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id F267816A40F for ; Wed, 8 Nov 2006 15:07:50 +0000 (UTC) (envelope-from jkim@FreeBSD.org) Received: from anuket.mj.niksun.com (gwnew.niksun.com [65.115.46.162]) by mx1.FreeBSD.org (Postfix) with ESMTP id 76B9C43D5E for ; Wed, 8 Nov 2006 15:07:50 +0000 (GMT) (envelope-from jkim@FreeBSD.org) Received: from niksun.com (anuket [10.70.0.5]) by anuket.mj.niksun.com (8.13.1/8.13.1) with ESMTP id kA8F7Tvs008152; Wed, 8 Nov 2006 10:07:30 -0500 (EST) (envelope-from jkim@FreeBSD.org) From: Jung-uk Kim To: freebsd-emulation@FreeBSD.org Date: Wed, 8 Nov 2006 10:06:56 -0500 User-Agent: KMail/1.6.2 References: <20061106174033.GA70360@stud.fit.vutbr.cz> <200611071237.11856.jkim@FreeBSD.org> <20061108104309.H55159@delplex.bde.org> In-Reply-To: <20061108104309.H55159@delplex.bde.org> MIME-Version: 1.0 Content-Disposition: inline Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <200611081007.01759.jkim@FreeBSD.org> X-Virus-Scanned: ClamAV 0.88.6/2176/Wed Nov 8 06:53:41 2006 on anuket.mj.niksun.com X-Virus-Status: Clean Cc: Subject: Re: [PATCH]: possible fix for the fifoor problem X-BeenThere: freebsd-emulation@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Development of Emulators of other operating systems List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 08 Nov 2006 15:07:51 -0000 On Tuesday 07 November 2006 07:16 pm, Bruce Evans wrote: > On Tue, 7 Nov 2006, Jung-uk Kim wrote: > > On Tuesday 07 November 2006 12:19 pm, Divacky Roman wrote: > >> the patch is wrong.... this forces NONBLOCKing on all opened > >> files which is wrong. > > > > Nope. It does not force anything. > > But it does. open() has side effects for some file types, and does > have signifcant side effects for fifos. The original problem is > caused by the side effect of blocking. Opening for writing or > read/write would have the side effect of unblocking all readers > blocked in open() and letting their open() complete. Opening for > reading gives more delicate state changes which might be harmless, > but this is not clear. Ah, you are correct. I had to think more thoroughly. > > static void > > translate_path_major_minor(struct thread *td, char *path, struct > > stat *buf) { > > struct proc *p = td->td_proc; > > struct filedesc *fdp = p->p_fd; > > struct file *fp; > > int fd; > > int temp; > > > > temp = td->td_retval[0]; > > if (kern_open(td, path, UIO_SYSSPACE, O_RDONLY, 0) != 0) > > return; > > fd = td->td_retval[0]; > > td->td_retval[0] = temp; > > translate_fd_major_minor(td, fd, buf); > > FILEDESC_LOCK(fdp); > > fp = fdp->fd_ofiles[fd]; > > FILEDESC_UNLOCK(fdp); > > fdclose(fdp, fdp->fd_ofiles[fd], fd, td); > > } > > > > As you can see the function is only used internally to convert > > major/minor and fd is closed at the end of the function. > > Anything more than a stat() is too dangerous. Yeah, it is really ugly hack already. > >> according to a comment in linux source code linux never blocks > >> for O_RDWR which is what I tried to implement with my patch > > > > We don't advertise it but we do the same, AFAIK. ;-) > > I think this happens automatically. The reader would block without > O_NONBLOCK or a writer, but O_RDWR gives a writer so the only > possible block is a transient internal one if the implementation > opens the reader first. The thread doing the O_RDWR open() > couldn't see this, but other threads might be able to see it if the > implementation doesn't hold a lock throughout the open(). > > But utility functions cannot use O_RDWR due to its side effect. Thanks for the explanation. > BTW, I never got around to committing the following workaround for > a related bug in freopen(), since I'm not happy with the patch > though I have been using it for about 10 years: > > %%% > Index: freopen.c > =================================================================== > RCS file: /home/ncvs/src/lib/libc/stdio/freopen.c,v > retrieving revision 1.13 > diff -u -2 -r1.13 freopen.c > --- freopen.c 22 May 2004 15:19:41 -0000 1.13 > +++ freopen.c 23 May 2004 04:01:46 -0000 > @@ -64,4 +64,5 @@ > FILE *fp; > { > + struct stat st; > int f; > int dflags, flags, isopen, oflags, sverrno, wantfd; > @@ -137,4 +138,7 @@ > * a descriptor, defer closing it; freopen("/dev/stdin", "r", > stdin) * should work. This is unnecessary if it was not a Unix > file. + * However, not closing the descriptor is a bug if closing > it would + * have side effects. We close fifos because completely > closing a + * fifo flushes it, and the NIST POSIX test suite tests > for this. */ > if (fp->_flags == 0) { > @@ -148,5 +152,9 @@ > /* if close is NULL, closing is a no-op, hence pointless */ > isopen = fp->_close != NULL; > - if ((wantfd = fp->_file) < 0 && isopen) { > + wantfd = fp->_file; > + if (isopen && > + (wantfd < 0 || > + (_fstat(wantfd, &st) == 0 && S_ISFIFO(st.st_mode) > + && st.st_ino != 0))) { > (void) (*fp->_close)(fp->_cookie); > isopen = 0; > %%% > > stat() can be used similarly to give a workaround (that I'm not > happy with) for translate_path_major_minor(): first stat() the > path, and don't do anything unless it is a cdev or maybe a bdev > (translate_fd_major_minor() still supports bdevs). This avoids the > problem for fifos. However, cdevs must be handled, and opening of > cdevs can have harmmful side effects (e.g., rewinding of tape > drives). My fix for freopen() is incomplete because it doesn't > handle the opposite problem. E.g., the missing close() might break > setting of file marks on tapes. The problem is just smaller for > freopen() since stdio shouldn't be used on tape drives, etc., while > the translation function is used a lot and doesn't require much > privilege. Interesting... I knew the proper fix would be harder than my one-line patch. ;-) Thanks, Jung-uk Kim