cleanup of cbrt() handling

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