* Observations on select.cc (19990922 snapshot)
@ 1999-09-24 13:42 Patrick J. LoPresti
1999-09-25 10:31 ` Chris Faylor
1999-09-30 23:42 ` Patrick J. LoPresti
0 siblings, 2 replies; 6+ messages in thread
From: Patrick J. LoPresti @ 1999-09-24 13:42 UTC (permalink / raw)
To: cygwin
Just some observations from reading the code and playing with strace.
Pull up select.cc and follow along.
The peek_pipe() function frequently gets called on the input side of a
pipe. That causes the call to PeekNamedPipe() to fail with a
permission error (Win32 only lets you peek at the output side), which
is kind of annoying. Perhaps peek_pipe() could check read_selected
before doing the PeekNamedPipe?
Also, there currently seem to be only clunky attempts to enforce the
invariant that the bits set in the fd_sets when select() returns are
subsets of the bits which were passed in. (I'm not even certain this
is always true.) Wouldn't it make sense for the set_bits() function
to enforce this invariant by checking {read,write,except}_selected
before setting the corresponding bit? Then many of the read_selected
tests around the file could be removed.
By the way, I am not asking anyone else to do this work (unless they
want to). I just want to confirm with the developers that these are
reasonable things to do before I bother...
Thanks!
- Pat
--
Want to unsubscribe from this list?
Send a message to cygwin-unsubscribe@sourceware.cygnus.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Observations on select.cc (19990922 snapshot)
1999-09-24 13:42 Observations on select.cc (19990922 snapshot) Patrick J. LoPresti
@ 1999-09-25 10:31 ` Chris Faylor
1999-09-25 10:55 ` Patrick J. LoPresti
1999-09-30 23:42 ` Chris Faylor
1999-09-30 23:42 ` Patrick J. LoPresti
1 sibling, 2 replies; 6+ messages in thread
From: Chris Faylor @ 1999-09-25 10:31 UTC (permalink / raw)
To: Patrick J. LoPresti; +Cc: cygwin
On Fri, Sep 24, 1999 at 04:40:20PM -0400, Patrick J. LoPresti wrote:
>Just some observations from reading the code and playing with strace.
>Pull up select.cc and follow along.
>
>The peek_pipe() function frequently gets called on the input side of a
>pipe. That causes the call to PeekNamedPipe() to fail with a
>permission error (Win32 only lets you peek at the output side), which
>is kind of annoying. Perhaps peek_pipe() could check read_selected
>before doing the PeekNamedPipe?
Looking at a recent strace output, I don't see any occurrences of
PeekNamedPipe failures. So, I'm not sure where this is frequently being
called.
PeekNamedPipe is supposed to be called with a pipe handle which has
GENERIC_READ attributes. I don't know what you consider the output side
of a pipe but I wouldn't expect an output handle to have GENERIC_READ
attributes.
>Also, there currently seem to be only clunky attempts to enforce the
>invariant that the bits set in the fd_sets when select() returns are
>subsets of the bits which were passed in. (I'm not even certain this
>is always true.) Wouldn't it make sense for the set_bits() function
>to enforce this invariant by checking {read,write,except}_selected
>before setting the corresponding bit? Then many of the read_selected
>tests around the file could be removed.
Hmm. I assume that this paragraph is your "clunky" way of trying to
start a technical discussion. You do realize that when you send mail
here mailing list you stand a chance of communicating with the original
author of the code, right? I'll grant you that the word "clunky" did
get my attention but certainly not in a positive way.
However, you're right that the *_selected test should be in set_bits. I
don't agree that all of the *_selected tests should be eliminated
elsewhere, however.
-chris
--
Want to unsubscribe from this list?
Send a message to cygwin-unsubscribe@sourceware.cygnus.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Observations on select.cc (19990922 snapshot)
1999-09-25 10:31 ` Chris Faylor
@ 1999-09-25 10:55 ` Patrick J. LoPresti
1999-09-30 23:42 ` Patrick J. LoPresti
1999-09-30 23:42 ` Chris Faylor
1 sibling, 1 reply; 6+ messages in thread
From: Patrick J. LoPresti @ 1999-09-25 10:55 UTC (permalink / raw)
To: Chris Faylor; +Cc: cygwin
>>>>> "cgf" == Chris Faylor <cgf@cygnus.com> writes:
cgf> Looking at a recent strace output, I don't see any occurrences
cgf> of PeekNamedPipe failures. So, I'm not sure where this is
cgf> frequently being called.
It happens any time anyone does a select() to see if they can write to
a pipe. The "always_ready" test in select() succeeds, and select()
then calls sellect_stuff::poll(). This loops through all of the
select records, calling s->poll() on each. For a pipe, this means
calling poll_pipe() as decided by fhandler_pipe::select_write() (or
fhandler_pipe::select_read() if the pipe was in both the read and
write fd_sets).
cgf> PeekNamedPipe is supposed to be called with a pipe handle which
cgf> has GENERIC_READ attributes. I don't know what you consider the
cgf> output side of a pipe but I wouldn't expect an output handle to
cgf> have GENERIC_READ attributes.
By "output side" I meant the side of the pipe out of which data are
coming. Sorry for being ambiguous.
cgf> Hmm. I assume that this paragraph is your "clunky" way of
cgf> trying to start a technical discussion. You do realize that
cgf> when you send mail here mailing list you stand a chance of
cgf> communicating with the original author of the code, right? I'll
cgf> grant you that the word "clunky" did get my attention but
cgf> certainly not in a positive way.
Sorry; I meant no offense! I did not mean that the code is bad. I
just meant that when I tried to convince myself that this particular
invariant was enforced, I found it difficult; and at the same time, I
saw a way to make it easy. I will try to use less loaded words when
describing trivial problems.
cgf> However, you're right that the *_selected test should be in
cgf> set_bits. I don't agree that all of the *_selected tests should
cgf> be eliminated elsewhere, however.
Personally, I would either enforce the invariant everywhere and then
*ASSERT* it in set_bits(); or I would enforce it in set_bits() and not
bother elsewhere. Naturally, your tastes may differ :-).
- Pat
--
Want to unsubscribe from this list?
Send a message to cygwin-unsubscribe@sourceware.cygnus.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Observations on select.cc (19990922 snapshot)
1999-09-25 10:31 ` Chris Faylor
1999-09-25 10:55 ` Patrick J. LoPresti
@ 1999-09-30 23:42 ` Chris Faylor
1 sibling, 0 replies; 6+ messages in thread
From: Chris Faylor @ 1999-09-30 23:42 UTC (permalink / raw)
To: Patrick J. LoPresti; +Cc: cygwin
On Fri, Sep 24, 1999 at 04:40:20PM -0400, Patrick J. LoPresti wrote:
>Just some observations from reading the code and playing with strace.
>Pull up select.cc and follow along.
>
>The peek_pipe() function frequently gets called on the input side of a
>pipe. That causes the call to PeekNamedPipe() to fail with a
>permission error (Win32 only lets you peek at the output side), which
>is kind of annoying. Perhaps peek_pipe() could check read_selected
>before doing the PeekNamedPipe?
Looking at a recent strace output, I don't see any occurrences of
PeekNamedPipe failures. So, I'm not sure where this is frequently being
called.
PeekNamedPipe is supposed to be called with a pipe handle which has
GENERIC_READ attributes. I don't know what you consider the output side
of a pipe but I wouldn't expect an output handle to have GENERIC_READ
attributes.
>Also, there currently seem to be only clunky attempts to enforce the
>invariant that the bits set in the fd_sets when select() returns are
>subsets of the bits which were passed in. (I'm not even certain this
>is always true.) Wouldn't it make sense for the set_bits() function
>to enforce this invariant by checking {read,write,except}_selected
>before setting the corresponding bit? Then many of the read_selected
>tests around the file could be removed.
Hmm. I assume that this paragraph is your "clunky" way of trying to
start a technical discussion. You do realize that when you send mail
here mailing list you stand a chance of communicating with the original
author of the code, right? I'll grant you that the word "clunky" did
get my attention but certainly not in a positive way.
However, you're right that the *_selected test should be in set_bits. I
don't agree that all of the *_selected tests should be eliminated
elsewhere, however.
-chris
--
Want to unsubscribe from this list?
Send a message to cygwin-unsubscribe@sourceware.cygnus.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* Observations on select.cc (19990922 snapshot)
1999-09-24 13:42 Observations on select.cc (19990922 snapshot) Patrick J. LoPresti
1999-09-25 10:31 ` Chris Faylor
@ 1999-09-30 23:42 ` Patrick J. LoPresti
1 sibling, 0 replies; 6+ messages in thread
From: Patrick J. LoPresti @ 1999-09-30 23:42 UTC (permalink / raw)
To: cygwin
Just some observations from reading the code and playing with strace.
Pull up select.cc and follow along.
The peek_pipe() function frequently gets called on the input side of a
pipe. That causes the call to PeekNamedPipe() to fail with a
permission error (Win32 only lets you peek at the output side), which
is kind of annoying. Perhaps peek_pipe() could check read_selected
before doing the PeekNamedPipe?
Also, there currently seem to be only clunky attempts to enforce the
invariant that the bits set in the fd_sets when select() returns are
subsets of the bits which were passed in. (I'm not even certain this
is always true.) Wouldn't it make sense for the set_bits() function
to enforce this invariant by checking {read,write,except}_selected
before setting the corresponding bit? Then many of the read_selected
tests around the file could be removed.
By the way, I am not asking anyone else to do this work (unless they
want to). I just want to confirm with the developers that these are
reasonable things to do before I bother...
Thanks!
- Pat
--
Want to unsubscribe from this list?
Send a message to cygwin-unsubscribe@sourceware.cygnus.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Observations on select.cc (19990922 snapshot)
1999-09-25 10:55 ` Patrick J. LoPresti
@ 1999-09-30 23:42 ` Patrick J. LoPresti
0 siblings, 0 replies; 6+ messages in thread
From: Patrick J. LoPresti @ 1999-09-30 23:42 UTC (permalink / raw)
To: Chris Faylor; +Cc: cygwin
>>>>> "cgf" == Chris Faylor <cgf@cygnus.com> writes:
cgf> Looking at a recent strace output, I don't see any occurrences
cgf> of PeekNamedPipe failures. So, I'm not sure where this is
cgf> frequently being called.
It happens any time anyone does a select() to see if they can write to
a pipe. The "always_ready" test in select() succeeds, and select()
then calls sellect_stuff::poll(). This loops through all of the
select records, calling s->poll() on each. For a pipe, this means
calling poll_pipe() as decided by fhandler_pipe::select_write() (or
fhandler_pipe::select_read() if the pipe was in both the read and
write fd_sets).
cgf> PeekNamedPipe is supposed to be called with a pipe handle which
cgf> has GENERIC_READ attributes. I don't know what you consider the
cgf> output side of a pipe but I wouldn't expect an output handle to
cgf> have GENERIC_READ attributes.
By "output side" I meant the side of the pipe out of which data are
coming. Sorry for being ambiguous.
cgf> Hmm. I assume that this paragraph is your "clunky" way of
cgf> trying to start a technical discussion. You do realize that
cgf> when you send mail here mailing list you stand a chance of
cgf> communicating with the original author of the code, right? I'll
cgf> grant you that the word "clunky" did get my attention but
cgf> certainly not in a positive way.
Sorry; I meant no offense! I did not mean that the code is bad. I
just meant that when I tried to convince myself that this particular
invariant was enforced, I found it difficult; and at the same time, I
saw a way to make it easy. I will try to use less loaded words when
describing trivial problems.
cgf> However, you're right that the *_selected test should be in
cgf> set_bits. I don't agree that all of the *_selected tests should
cgf> be eliminated elsewhere, however.
Personally, I would either enforce the invariant everywhere and then
*ASSERT* it in set_bits(); or I would enforce it in set_bits() and not
bother elsewhere. Naturally, your tastes may differ :-).
- Pat
--
Want to unsubscribe from this list?
Send a message to cygwin-unsubscribe@sourceware.cygnus.com
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~1999-09-30 23:42 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
1999-09-24 13:42 Observations on select.cc (19990922 snapshot) Patrick J. LoPresti
1999-09-25 10:31 ` Chris Faylor
1999-09-25 10:55 ` Patrick J. LoPresti
1999-09-30 23:42 ` Patrick J. LoPresti
1999-09-30 23:42 ` Chris Faylor
1999-09-30 23:42 ` Patrick J. LoPresti
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).