This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

Author vstinner
Recipients giampaolo.rodola, rosslagerwall, vstinner
Date 2011-06-24.19:32:52
SpamBayes Score 3.330669e-16
Marked as misclassified No
Message-id <1308943973.64.0.824776448347.issue12303@psf.upfronthosting.co.za>
In-reply-to
Content
>> 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
History
Date User Action Args
2011-06-24 19:32:53vstinnersetrecipients: + vstinner, giampaolo.rodola, rosslagerwall
2011-06-24 19:32:53vstinnersetmessageid: <1308943973.64.0.824776448347.issue12303@psf.upfronthosting.co.za>
2011-06-24 19:32:53vstinnerlinkissue12303 messages
2011-06-24 19:32:52vstinnercreate