Обсуждение: Re: [HACKERS] (: JDBC+(Sun ~3:pm MST) CVS :) -also question about regression tests

"Thomas G. Lockhart" wrote:
  >The other good possibility is for Oliver to develop a patch kit (he has done
      > so
  >already) which we can include in the v6.3 distribution to be applied only to
  >linux/glibc2. When the beta settles down perhaps Oliver can generate a new p
      >atch
  >kit which we can include?

I'm happy to remake the patch in a week or so.  However I'd like to be clear
what you think is 'Linux/glibc-specific' (and therefore undesirable); obviously
it is better to make changes as general as possible.

I'm attaching the previous patch I made (which, of course, may not
actually work with the current release - I haven't tried yet) so as to comment
on it:

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? ]

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.

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.
Oliver Elphick                                Oliver.Elphick@lfix.co.uk
Isle of Wight                              http://www.lfix.co.uk/oliver

PGP key from public servers; key ID 32B8FAA1

> 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