[Patches] [Patch #100852] Add poll() to select module

noreply@sourceforge.net noreply@sourceforge.net
Tue, 15 Aug 2000 10:36:01 -0700


Patch #100852 has been updated. 

Project: 
Category: None
Status: Open
Summary: Add poll() to select module

Follow-Ups:

Date: 2000-Jul-11 04:25
By: akuchling

Comment:
Derived from Sam Rushing's pollmodule.c that forms
part of Medusa.  Once I get a second opinion about the patch, I'll check it in.


-------------------------------------------------------

Date: 2000-Jul-21 06:33
By: akuchling

Comment:
Updated version of the patch, which adds poll() to the select module.
.register() and .unregister() add/remove fds, and .poll() performs the system call.  

Open question: what should register/unregister do if you attempt to register a fd twice?  Or remove a fd that isn't registered?  In this version, 
no exception is raised.  IMHO the second case should definitely be an error (ValueError?).  I'm not sure about the first case, but maybe it should continue to be ignored.

Open question: should there be a function or attribute to get the list of 
registered fds?

Still to do: write documentation, add the test case.

-------------------------------------------------------

Date: 2000-Jul-24 17:54
By: akuchling

Comment:
Assigned to Jeremy for review; feel free to bounce it to someone else.
-------------------------------------------------------

Date: 2000-Jul-25 10:25
By: akuchling

Comment:
Update patch to latest version, which adds poll() to the select() module.
-------------------------------------------------------

Date: 2000-Jul-25 13:30
By: jhylton

Comment:
Several style nits changed, mostly to do with whitespace.

Substantive changes:
1. include poll.h instead of sys/poll.h; this works on Linux and is recommended by Stevens
2. add symbolic constants for POLLIN, ..., POLLNVAL, ... POLLWRBAND

One more Open Issue:
I worry about the linear search and realloc that occur every time a fd is registered or unregistered.  Possible solutions:
- interface to register many fds at once
- keep the list of fds in sorted order
- use a dict to store fds and generate fdarry lazily
- keep track of allocated length and used length for ufds, so that fewer reallocs are required
Each of these suggestions has some added complexity, so I'm not sure that I like any of them.

-------------------------------------------------------

Date: 2000-Jul-25 13:41
By: akuchling

Comment:
I wondered about the linear search, too; it means balancing 
efficiency versus the lines of code to write this one object?  IMHO implementing both #1 and #3 would be
good; allow passing an arbitrary # of fds to register (and
unregister?), store them in a dict, and then lazily generate the ufd
array when needed.  Or is there some built-in more suited than a dict for this?

Hmm... you could also return the dict as an attribute of the polling object, and modifying it directly would also register or remove descriptors.
-------------------------------------------------------

Date: 2000-Jul-25 14:40
By: jhylton

Comment:
I think a dict is the right data structure, but I don't think it should be exposed directly.  If it is not exposed, then the ufd array need only be generated for poll calls after a register or unregister.  If there are multiple polls without a change, the same array can be re-used.

-------------------------------------------------------

Date: 2000-Aug-12 21:13
By: akuchling

Comment:
Latest version, which stores an underlying dict as 
suggested in jhylton's previous comment.  

p.unregister() raises KeyError if you unregister a fd
that wasn't registered in the first place; seem reasonable?
It does to me...

The code should now be completed; the only remaining task
is to write docstrings and finish the documentation patch.


-------------------------------------------------------

Date: 2000-Aug-15 10:36
By: tim_one

Comment:
Tickling you as part of 2.0 nagging.  Please leave a note saying when you intend to "write docstrings and finish the documentation".
-------------------------------------------------------

-------------------------------------------------------
For more info, visit:

http://sourceforge.net/patch/?func=detailpatch&patch_id=100852&group_id=5470