Re: [HACKERS] (: JDBC+(Sun ~3:pm MST) CVS :) -also question about regression tests - Mailing list pgsql-hackers

From Thomas G. Lockhart
Subject Re: [HACKERS] (: JDBC+(Sun ~3:pm MST) CVS :) -also question about regression tests
Date
Msg-id 34D733DE.4665F9FB@alumni.caltech.edu
Whole thread Raw
In response to Re: [HACKERS] (: JDBC+(Sun ~3:pm MST) CVS :) -also question about regression tests  ("Oliver Elphick" <olly@lfix.co.uk>)
List pgsql-hackers
> 1.  I use `#if __GLIBC__ < 2' to test for glibc, where the change is only
>     for glibc.  So I use it to force HAVE_INT_TIMEZONE to be undefined
>     and to redefine TMODULO(); incidentally, I used the hint in the existing
>     source that modf() was sometimes broken and chose to use glibc's
>     unbroken modf().
>
>     [ Incidentally, is modf() _still_ broken on whatever platforms it was
>     broken on? ]

Probably, since it was on Solaris, which should have been mature enough by now to not exhibit the problem :(

> 2.  The second set of changes are to do with unnecessary printing of n.00
>     instead of n, where the test was whether fsec is non-zero.  With the
>     glibc version, fsec is often non-zero but integer, so I substituted
>     the test (fsec != rint(fsec)) for (fsec != 0).  This seems to me to
>     be a completely general change that will work correctly on all
>     implementations.

The second set of changes does work in general, but is an extra math library call which is _only_ required for glibc2.
This
is the math library bug I was alluding to earlier.

The extra overhead of this unnecessary function call should not be required for all platforms, since it only there as a
bug
fix workaround for one platform. "fsec" is "fractional seconds", not "fractional seconds except for glibc2 where it
couldbe 
an integer too".

> Now I can understand that you may choose to ignore the existence of glibc
> for the standard code (not that I agree, mind you) but I really don't see
> how I could make the changes any more general than I have.

Heh, don't get me wrong, I _love_ glibc2 and am looking forward to being able to write thread-safe code using it.
However,
the glibc2 you are using seems to be v2.0.5 or thereabouts and it still apparently has a few rough edges, this math
rounding
problem being one of them.

As you know, the problem is not really with rint() but with whatever allowed the fsec variable to become a non-zero
integer.
_That_ is the thing which should be patched with glibc2-specific #ifdefs. Between all other tested platforms (~10?) the
behavior is uniform and correct. So, I'll be at least somewhat happier with the patches if you identify the place where
fsec
is going bad and fix that for glibc2, rather than coding a general workaround later in the code.

The more I think about it, the more I want to go back and rip out the general workaround I put in to fix Solaris. Then,
we
can isolate up in the top of the file some #ifdef equivalent functions for Solaris and for glibc2.

Someday the glibc2 behavior will be fixed, and it should be easy to then remove the workaround code.

                                                - Tom


pgsql-hackers by date:

Previous
From: The Hermit Hacker
Date:
Subject: Re: [HACKERS] (: JDBC+(Sun ~3:pm MST) CVS :) -also question about regression tests
Next
From: Shiby Thomas
Date:
Subject: Execution time.