Hi, Dave -
> The following commit implements a global timeout for
> __pmDiscoverServicesWithOptions().
Looks good.
> The implementation is a sleeping thread which wakes up after the
> specified amount of time and sets a flag indicating that the timeout
> has expired.
That sounds good. The lack of fallback for non-threaded libpcp's
could be a problem; have you considered returning an outright error
instead of the "__pmNotifyErr() & return OK" in this case?
In __pmDiscoverServicesWithOptions(), during the thread cleanup,
it looks like the code cares about the sts of the pthread_cancel(),
but doesn't really. -ESRCH is a valid non-error case there, FWIW.
Consider just a "(void) pthread_cancel(...);".
For the __pmServiceDiscoveryOptions timedOut field, did you consider
having the thread set the INTERRUPTED bit in the flags, instead of
that new field? If so, good job figuring out that it'd be a bad idea :-)
due to the need to eschew having libpcp modify that const * flags.
(All that C options-processing string/realloc() stuff in pmfind.c must
leave one, in weak moments, covetous of C++.)
- FChE
|