On Wed, 2010-01-13 at 16:54 -0500, Stephen Gallagher wrote:
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 01/13/2010 04:11 PM, Martin Nagy wrote:
> This fixes #381. This patch is based on Steven's DNS timeout patch.
>
> Martin
>
Nack (but comments are minor)
I recommend the following (simplified) rewrite of get_ares_timeout_time().
static void
get_ares_timeout_time(struct resolv_ctx *ctx, struct timeval *tv)
{
struct timeval tv_max;
struct timeval *tvp;
tv_max.tv_sec = ctx->timeout + 1;
tv_max.tv_usec = 0;
tvp = ares_timeout(ctx->channel, &tv_max, &tv_max);
/* Enforce a minimum of 1 second. */
if (tvp->tv_sec < 1) {
*tv = tevent_timeval_current_ofs(1, 0);
return;
}
*tv = tevent_timeval_current_ofs(tvp->tv_sec, tvp->tv_usec);
}
Then replace current_time in check_fd_timeouts() and eliminate
gettimeofday() in schedule_timeout_watcher()
I went ahead and also moved tevent_add_timer() in there to eliminate
even more duplication and I renamed the function.
tevent_timeval_current_ofs() is our preferred way to set timer
offsets.
Also, I'm not certain about the use of setting tv_max. According to the
ares_timeout documentation, that's only useful to clamp it down to
something shorter than the timeout we set originally. If we're using
ctx->timeout+1, it's functionally identical to using NULL here (and more
confusing).
Well, in any case, ares_timeout() can return NULL this way, without
really setting the buffer. But I think this is rare, if at all possible,
but added a check to be sure.
Thanks for all your comments.
Martin