Message138976
>> It is possible to pass a negative timeout
> It now raises an exception like select.
Great.
>> According to the manual page, sigwaitinfo() or sigtimedwait()
>> can be interrupted (EINTR)
> Actually, PyErr_SetFromErrno() does this implicitly. I added a test case anyway.
I would prefer an explicit call to PyErr_CheckSignals(), but if there is a test, it's just fine.
> So I've left out si_addr (I don't think it's needed)
> ... and si_value (not sure what it's for or how to represent a union :-))
I don't think that it means something in Python to have an address or this low-level "value" field. If someone needs them, we would need an use case with an example (to test it!). (So ok to not expose them)
> ... si_band (we don't have SIGPOLL)
What do you mean? signal.SIGPOLL exists in Python 3.3.
>> sigtimedwait() raises a OSError(EGAIN) on timeout.
> It now returns None.
Great.
--
>> I will do a deeper review on the second version of your patch :-)
> How much more in depth can it get ;-) ?
Here you have, issue12303_v2.patch:
- "PyStructSequence_SET_ITEM(result, 4, PyLong_FromPid(si->si_uid));" looks wrong (we don't need to call PyLong_FromLongLong): si_uid type is uid_t, not pid_t. posix_getuid() simply uses PyLong_FromLong((long)getuid()).
- sigwaitinfo() doc doesn't mention that the function can be interrupted if an unexpected signal is received. I don't know if it should be mentionned. It is mentionned in the manpage, in the ERRORS section (EINTR).
- tests: test_sigtimedwait_negative_timeout() doesn't need to use _wait_helper() (which creates a subprocess!). You may also test (-1, 0) and (0, -1) timeouts.
- test_sigwaitinfo(), test_sigtimedwait_poll(), test_sigwaitinfo_interrupted() are called from a child process. In test_wait(), I chose to write manually to stdout and call os._exit(1) (oh, I forgot an explicit sys.stdout.flush()). I don't know if you can use TestCase methods in a child process (I don't know what is written to stdout, _wait_helper() calls os._exit).
- you may want to prepare the "What's new in Python 3.3" document (mention the 2 new functions)
- style: _wait_helper(): you don't have to mark the helper as private (-> wait_helper())
- style: _fill_siginfo() is already a static function you don't need a "_" prefix |
|
Date |
User |
Action |
Args |
2011-06-24 19:32:53 | vstinner | set | recipients:
+ vstinner, giampaolo.rodola, rosslagerwall |
2011-06-24 19:32:53 | vstinner | set | messageid: <1308943973.64.0.824776448347.issue12303@psf.upfronthosting.co.za> |
2011-06-24 19:32:53 | vstinner | link | issue12303 messages |
2011-06-24 19:32:52 | vstinner | create | |
|