> So I've reviewed the v6 patch, and here are some comments:
>
> wait.c no longer calls rint() or isnan(), so we don't need to include math.h.
>
> ---
> +          The valid range is <literal>0 .. INT_MAX</literal> milliseconds.
> +          A value of zero means waiting indefinitely.
>
> INT_MAX is a C identifier and it's better to avoid using it in that section.
>
> Also, the current description "The timeout might be given as integer
> number of milliseconds." seems not correct to me since pares_int()
> falls back to strtod() when the value has a decimal point. ANd the
> paragraph also doesn't mention the rounding. How about rewriting it to
> something like:
>
>           <para>
>            When specified and <parameter>timeout</parameter> is
> greater than zero,
>            the command waits until <parameter>lsn</parameter> is reached or
> -          the specified <parameter>timeout</parameter> has elapsed.
> +          the specified <parameter>timeout</parameter> has elapsed.  A value
> +          of zero (the default) means the command waits indefinitely.
>           </para>
>           <para>
> -          The <parameter>timeout</parameter> might be given as
> integer number of
> -          milliseconds.  Also it might be given as string literal with
> -          integer number of milliseconds or a number with unit
> -          (see <xref linkend="config-setting-names-values"/>).
> +          The <parameter>timeout</parameter> is an amount of time in
> +          milliseconds.  It may also be specified as a string containing the
> +          numerical value followed by a time unit
> +          (see <xref linkend="config-setting-names-values"/>).  The maximum
> +          value is <literal>2147483647 ms</literal>.
> +         </para>
> +         <para>
> +          Fractional values are rounded to the nearest millisecond.  Note
> +          that a <parameter>timeout</parameter> of half a millisecond or
> +          less therefore rounds down to zero, which means waiting
> +          indefinitely.
>           </para>
>
> I've attached the updated patch that incorporated the above points.
> Please review it.

Thanks for updating/reviewing the patch!

LGTM.

> > I still think that it needs to be fixed in HEAD.
>
> Agreed, and I'd rather we did that on its own thread.

OK. I'll start a new thread for it when the somewhat overwhelmed v19
clean-up phase is over. I'd also be happy to help if fellow hackers
passing by decide to tackle it.

-- 
Regards,
Xuneng Zhou
HighGo Software Co., Ltd.


Reply via email to