Обсуждение: cleanup of cbrt() handling
This patch cleans up our static cbrt() implementation in float.c. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073 Index: src/backend/utils/adt/float.c =================================================================== RCS file: /cvsroot/pgsql-server/src/backend/utils/adt/float.c,v retrieving revision 1.87 diff -c -c -r1.87 float.c *** src/backend/utils/adt/float.c 9 May 2003 21:19:49 -0000 1.87 --- src/backend/utils/adt/float.c 25 May 2003 05:28:58 -0000 *************** *** 70,93 **** #include "utils/builtins.h" - #if !(NeXT && NX_CURRENT_COMPILER_RELEASE > NX_COMPILER_RELEASE_3_2) - /* NS3.3 has conflicting declarations of these in <math.h> */ - - #ifndef atof - extern double atof(const char *p); - #endif - #ifndef HAVE_CBRT - #define cbrt my_cbrt static double cbrt(double x); - - #else - #if !defined(nextstep) - extern double cbrt(double x); - #endif #endif /* HAVE_CBRT */ - #endif /* NeXT check */ - #ifndef M_PI /* from my RH5.2 gcc math.h file - thomas 2000-04-03 */ --- 70,78 ---- *************** *** 1983,1989 **** /* ========== PRIVATE ROUTINES ========== */ #ifndef HAVE_CBRT ! static double cbrt(double x) { --- 1968,1974 ---- /* ========== PRIVATE ROUTINES ========== */ #ifndef HAVE_CBRT ! /* I doubt this is still needed by any platform. 2003-05-25 */ static double cbrt(double x) {
Bruce Momjian <pgman@candle.pha.pa.us> writes: > This patch cleans up our static cbrt() implementation in float.c. Looks like an improvement to me. regards, tom lane
Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > This patch cleans up our static cbrt() implementation in float.c. > > Looks like an improvement to me. Yep, however, now that I look at the original code, and code that shipped in 7.3: #ifndef HAVE_CBRT #define cbrt my_cbrt static double cbrt(double x); #else #if !defined(nextstep) extern double cbrt(double x); #endif #endif /* HAVE_CBRT */ There is no my_cbrt() function, meaning anyone who didn't have cbrt couldn't have even compiled 7.3, so I think we should just remove cbrt, or mark it as UNUSED. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
Bruce Momjian <pgman@candle.pha.pa.us> writes: > #ifndef HAVE_CBRT > #define cbrt my_cbrt > static double cbrt(double x); > #else > #if !defined(nextstep) > extern double cbrt(double x); > #endif > #endif /* HAVE_CBRT */ > There is no my_cbrt() function, meaning anyone who didn't have cbrt > couldn't have even compiled 7.3, so I think we should just remove > cbrt, No, you're misreading the point of the code. The #define changes the spelling of the static declaration. The idea evidently is to make sure that there is no conflict of the static function against a library cbrt(), on the off chance that configure missed finding it somehow. This might be overly tricky --- certainly we do not take comparable precautions for other library-substitute functions. I wouldn't object to removing the "#define cbrt my_cbrt". But you have *no* proof that removing the whole thing won't break some supported platform. regards, tom lane
Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > #ifndef HAVE_CBRT > > #define cbrt my_cbrt > > static double cbrt(double x); > > > #else > > #if !defined(nextstep) > > extern double cbrt(double x); > > #endif > > #endif /* HAVE_CBRT */ > > > There is no my_cbrt() function, meaning anyone who didn't have cbrt > > couldn't have even compiled 7.3, so I think we should just remove > > cbrt, > > No, you're misreading the point of the code. The #define changes the > spelling of the static declaration. The idea evidently is to make sure > that there is no conflict of the static function against a library > cbrt(), on the off chance that configure missed finding it somehow. > This might be overly tricky --- certainly we do not take comparable > precautions for other library-substitute functions. I wouldn't object > to removing the "#define cbrt my_cbrt". But you have *no* proof that > removing the whole thing won't break some supported platform. Oh, I see, it makes all cbrt cases by my_cbrt, including the function calls _and_ function definition. I already removed my_cbrt, so let's see how it works in 7.4. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073