Date: Mon, 17 Dec 2001 12:29:53 -0600 From: Alfred Perlstein <alfred@FreeBSD.org> To: Bruce Evans <bde@zeta.org.au> Cc: wollman@FreeBSD.org, net@FreeBSD.org, cvs-committers@FreeBSD.org, cvs-all@FreeBSD.org Subject: Re: fifo fix (Re: cvs commit: src/sys/fs/fifofs fifo_vnops.c) Message-ID: <20011217122953.G82439@elvis.mu.org> In-Reply-To: <20011217230720.N13599-100000@gamplex.bde.org>; from bde@zeta.org.au on Tue, Dec 18, 2001 at 12:09:42AM %2B1100 References: <20011215151025.A82439@elvis.mu.org> <20011217230720.N13599-100000@gamplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
* Bruce Evans <bde@zeta.org.au> [011217 07:10] wrote:
> On Sat, 15 Dec 2001, Alfred Perlstein wrote:
>
> > Can people take a look at this fix? It seems to dtrt, but I need feedback
> > here.
> >
> > It basically backs out my last two revisions and changes the hacks the
> > poll call to seemingly do the right thing.
> >
> > Index: fifo_vnops.c
> > ===================================================================
> > RCS file: /home/ncvs/src/sys/fs/fifofs/fifo_vnops.c,v
> > retrieving revision 1.57
> > diff -u -r1.57 fifo_vnops.c
> > --- fifo_vnops.c 12 Dec 2001 09:35:33 -0000 1.57
> > +++ fifo_vnops.c 15 Dec 2001 21:25:30 -0000
> > ...
> > @@ -455,13 +451,23 @@
> > } */ *ap;
> > {
> > struct file filetmp;
> > - int revents = 0;
> > + int s, revents = 0;
>
> New style bug: disordered declaration. (Old style bug: initialized in
> auto declaration.)
>
> >
> > if (ap->a_events & (POLLIN | POLLPRI | POLLRDNORM | POLLRDBAND)) {
> > + struct socket *so;
> > + int oflg;
>
> Style bugs: nested declarations.
I'll take care of these two.
> > +
> > filetmp.f_data = (caddr_t)ap->a_vp->v_fifoinfo->fi_readsock;
> > + so = (struct socket *)filetmp.f_data;
> > + s = splnet();
> > + oflg = so->so_state & SS_CANTRCVMORE;
> > + if (ap->a_vp->v_fifoinfo->fi_writers == 0)
> > + so->so_state &= ~SS_CANTRCVMORE;
> > if (filetmp.f_data)
> > revents |= soo_poll(&filetmp, ap->a_events, ap->a_cred,
> > ap->a_td);
> > + so->so_state |= oflg;
> > + splx(s);
>
> I'm not happy with frobbing the socket state. I suggest frobbing the
> events mask instead. Either use a flag to tell sopoll() to ignore
> SS_CANTRCVMORE, or use new events POLLIN_IGNORE_EOF and
> POLLRDNORM_IGNORE_EOF (convert the userland POLLIN/POLLRDNORM to these
> and change sopoll() to support them).
I'll consider that.
There's actually a bug here, I need to make sure 'so' isn't NULL
before I do this, sticking the frobbing under the 'if (filetmp.f_data)'
should fix that.
> > }
> > if (ap->a_events & (POLLOUT | POLLWRNORM | POLLWRBAND)) {
> > filetmp.f_data = (caddr_t)ap->a_vp->v_fifoinfo->fi_writesock;
> >
>
> Similar changes may be needed for kevent(). kevent() has a way to modify
> modify the usual watermark handling for reads, but doesn't have anything
> similar for EOFs, although it already has more complete EOF handling
> (filt_kqread() sets EV_EOF for EOFs, but sopoll() is missing support for
> POLLHUP).
I'll talk to jlemon about this.
--
-Alfred Perlstein [alfred@freebsd.org]
'Instead of asking why a piece of software is using "1970s technology,"
start asking why software is ignoring 30 years of accumulated wisdom.'
http://www.morons.org/rants/gpl-harmful.php3
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe cvs-all" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20011217122953.G82439>