homepage

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 rosslagerwall
Recipients giampaolo.rodola, rosslagerwall, vstinner
Date 2011年06月24日.13:28:47
SpamBayes Score 2.1116442e-12
Marked as misclassified No
Message-id <1308922129.65.0.335775008549.issue12303@psf.upfronthosting.co.za>
In-reply-to
Content
Thanks for the review.
> Why do you wait until the end of PyInit_signal() to set initialized to 1?
Fixed.
> If this variable is only present ... but I see that the init function of the posixmodule.c has also a static initialized variable.
I simply followed the same format of the posix module.
> "(If one of the signals in set is already pending for the calling thread, sigwaitinfo() will return immediately with information about that signal.)"
Added.
> "If both fields of this structure are specified as 0, a poll is performed: sigtimedwait() returns immediately, either with information about a signal that was pending for the caller, or with an error if none of the signals in set was pending."
Added.
> The manpage doesn't tell that the signal handler is not called, should we say it in the Python doc?
Added - let's rather be more explicit.
> Doc: you may add links between pause(), sigwait(), sigwaitinfo() and sigtimedwait() functions.
Added.
> The timeout is a tuple. Usually, Python uses float for timeout (e.g. see select.select). I suppose that you chose a tuple to keep the precision of the timespec structure. We may accept both types: (sec: int, nanosec: int) or sec: float. It would be nice to have the opinion of our time expect, Alexander Belopolsky.
The are some uses of this tuple that were added in #10812 (e.g. futimens) that was reviewed by Alexander.
However, there was a conflict about it at #11457.
I'd say for now, let's keep it as a tuple and if a decision is eventually made as to how to
represent nanosecond timestamps, we can change them all then.
> It is possible to pass a negative timeout
It now raises an exception like select.
> signal_sigwaitinfo() and signal_sigtimedwait() use iterable_to_sigset()
Fixed.
> According to the manual page, sigwaitinfo() or sigtimedwait() can be interrupted (EINTR)
Actually, PyErr_SetFromErrno() does this implicitly. I added a test case anyway.
> Your patch doesn't touch configure nor pyconfig.h.in, only configure.in.
I prefer to keep the generated changes out of the patch. When I commit, I'll run autoreconf.
> siginfo_t structure contains more fields, but I don't know if we need all of them. It can be improved later.
POSIX 2008 specifies:
int si_signo Signal number. 
int si_code Signal code. 
int si_errno If non-zero, an errno value associated with 
 this signal, as described in <errno.h>. 
pid_t si_pid Sending process ID. 
uid_t si_uid Real user ID of sending process. 
void *si_addr Address of faulting instruction. 
int si_status Exit value or signal. 
long si_band Band event for SIGPOLL. 
union sigval si_value Signal value.
So I've left out si_addr (I don't think it's needed), si_band (we don't have SIGPOLL)
and si_value (not sure what it's for or how to represent a union :-)).
> sigtimedwait() raises a OSError(EGAIN) on timeout.
It now returns None.
> test_sigtimedwait_timeout(): why do you call signal.alarm()?
Copy/paste error.
> You may also add a test for timeout=0, I suppose that it is a special timeout value.
Added.
> I will do a deeper review on the second version of your patch :-)
How much more in depth can it get ;-) ?
History
Date User Action Args
2011年06月24日 13:28:50rosslagerwallsetrecipients: + rosslagerwall, vstinner, giampaolo.rodola
2011年06月24日 13:28:49rosslagerwallsetmessageid: <1308922129.65.0.335775008549.issue12303@psf.upfronthosting.co.za>
2011年06月24日 13:28:49rosslagerwalllinkissue12303 messages
2011年06月24日 13:28:48rosslagerwallcreate

AltStyle によって変換されたページ (->オリジナル) /