This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] resolv: Allow new "timeout-ms" option


On Thu, Jun 21, 2012 at 9:48 AM, Paul Eggert <eggert@cs.ucla.edu> wrote:
>
> On 06/21/2012 06:26 AM, Paul Stewart wrote:
> > Allow an option to specify DNS timeout in milliseconds instead
> > of seconds.
>
> Some dumb questions:
>
> Why mess with milliseconds if we can do nanoseconds?
> (This would be for the future, when chromium gets fast. :-)


If that was a serious question, I think 3 orders of magnitude more
granularity would do for now. ?In general, I believe just one
additional digit of precision is all that was really necessary.

>
>
> >+# define RES_MINWAIT_MS ? ? ? ? ? ? ? 5 ? ? ? /* Minimum wait time, in milliseconds */
>
> Why impose this limit? ?Isn't that arbitrary?
> Shouldn't users be able to wait for 4 ms
> if they really want to?

Yes, it is arbitrary.  I could specify 1ms here instead, if that's
preferable.  Considering what's contemporary for ping response times
(and the fact that these haven't been changing for years), and that
the previous minimum was 20 times higher, I thought the value or 5ms
might server for a while.
>
> > + ? ? int seconds = 0;
> > + ? ? int milliseconds = 0;
>
> The first initialization is unnecessary. ?The second one can be
> moved to the 'else' branch that needs it; this is clearer.

As you wish.  I cringe before declaring something undefined lest
future, less-well-thought-out changes to the code base run across such
landmines.

>
> > + ? ? ? ? ? ? if (seconds < 0 ||
> > + ? ? ? ? ? ? ? ? (seconds == 0 && milliseconds < RES_MINWAIT_MS)) {
>
> Put the "||" at the start of the next line (usual glibc style).

Thanks.  Will do.

>
> > + ? ? } else {
> > + ? ? ? ? ? ? seconds = operation_time;
> > + ? ? ? ? ? ? if (seconds <= 0)
> > + ? ? ? ? ? ? ? ? ? ? seconds = 1;
> > + ? ? }
>
> Shouldn't the default timeout be the same regardless of
> whether RES_TIMEOUT_MS is specified? ?As things stand,
> the default timeout is 1 s without the flag, 5 ms with it;
> is that really intended?

I think so.  I don't intend on changing the minimum timeout for the
entirety of glibc.  Only if the caller chooses to use higher precision
does the behavior change.  I thought that was the safest way to go.

I'll send a v2 of this patch with 3 changes: (1) minimum timeout set
to 1ms (2) remove unwanted initialization (3) style change for "||"
operator.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]