Patch: make behavior of all versions of the "isinf" function be similar
Hello, hackers!
While I was searching for a function which checks doubles for
infinity, I discovered a function "isinf" in a file src/port/isinf.c
where one of three versions returns different value for "-inf" ("1"
instead of "-1") comparing to the other two.
It seems concrete values (not just "if isinf(...)") are checked only
in float.c in float4out and float8out, but I am going to check for
concrete values in my another patch.
For systems with HAVE_FPCLASS the function returns the same result for
both "+inf" and "-inf". I can't test it on my WS, but I found only one
man page[1]https://docs.oracle.com/cd/E36784_01/html/E36874/fpclass-3c.html -- Best regards, Vitaly Burovoy where system header file "ieeefp.h" must be included for
ability to use "fpclass" function.
I guess nothing will be broken if that version of the function returns
the same results for input values as the other two.
Proposed patch makes that behavior.
P.S.: Should the patch be added to the next CF?
[1]: https://docs.oracle.com/cd/E36784_01/html/E36874/fpclass-3c.html -- Best regards, Vitaly Burovoy
--
Best regards,
Vitaly Burovoy
Attachments:
isinf_v1.patchapplication/octet-stream; name=isinf_v1.patchDownload
diff --git a/src/port/isinf.c b/src/port/isinf.c
index 7e8aabc..84aeb74 100644
--- a/src/port/isinf.c
+++ b/src/port/isinf.c
@@ -30,6 +30,7 @@ isinf(double d)
switch (type)
{
case FP_NINF:
+ return -1;
case FP_PINF:
return 1;
default:
On Mon, Feb 1, 2016 at 8:13 AM, Vitaly Burovoy <vitaly.burovoy@gmail.com> wrote:
While I was searching for a function which checks doubles for
infinity, I discovered a function "isinf" in a file src/port/isinf.c
where one of three versions returns different value for "-inf" ("1"
instead of "-1") comparing to the other two.For systems with HAVE_FPCLASS the function returns the same result for
both "+inf" and "-inf". I can't test it on my WS, but I found only one
man page[1] where system header file "ieeefp.h" must be included for
ability to use "fpclass" function.
That's the case on OSX, but on Linux -1 is returned for -Inf, and 1 for +Inf.
I guess nothing will be broken if that version of the function returns
the same results for input values as the other two.
Proposed patch makes that behavior.
Actually, is there actually a reason to keep this file in the code
tree? Are there platforms that do not have isinf()? Even for Windows
environments using MSVC < 1800 this is emulated using _fpclass.
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Mon, Feb 1, 2016 at 2:38 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
Actually, is there actually a reason to keep this file in the code
tree? Are there platforms that do not have isinf()? Even for Windows
environments using MSVC < 1800 this is emulated using _fpclass.
Looking at what is in the buildfarm, I noticed that isinf is provided
everywhere. Attached is a patch. Thoughts?
--
Michael
Attachments:
port-isinf-remove.patchtext/x-diff; charset=US-ASCII; name=port-isinf-remove.patchDownload
diff --git a/configure b/configure
index 3dd1b15..c9dd5b8 100755
--- a/configure
+++ b/configure
@@ -12633,63 +12633,6 @@ cat >>confdefs.h <<_ACEOF
_ACEOF
-
-{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for isinf" >&5
-$as_echo_n "checking for isinf... " >&6; }
-if ${ac_cv_func_isinf+:} false; then :
- $as_echo_n "(cached) " >&6
-else
- cat confdefs.h - <<_ACEOF >conftest.$ac_ext
-/* end confdefs.h. */
-
-#include <math.h>
-double glob_double;
-
-int
-main ()
-{
-return isinf(glob_double) ? 0 : 1;
- ;
- return 0;
-}
-_ACEOF
-if ac_fn_c_try_link "$LINENO"; then :
- ac_cv_func_isinf=yes
-else
- ac_cv_func_isinf=no
-fi
-rm -f core conftest.err conftest.$ac_objext \
- conftest$ac_exeext conftest.$ac_ext
-fi
-{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_cv_func_isinf" >&5
-$as_echo "$ac_cv_func_isinf" >&6; }
-
-if test $ac_cv_func_isinf = yes ; then
-
-$as_echo "#define HAVE_ISINF 1" >>confdefs.h
-
-else
- case " $LIBOBJS " in
- *" isinf.$ac_objext "* ) ;;
- *) LIBOBJS="$LIBOBJS isinf.$ac_objext"
- ;;
-esac
-
- # Look for a way to implement a substitute for isinf()
- for ac_func in fpclass fp_class fp_class_d class
-do :
- as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
-ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var"
-if eval test \"x\$"$as_ac_var"\" = x"yes"; then :
- cat >>confdefs.h <<_ACEOF
-#define `$as_echo "HAVE_$ac_func" | $as_tr_cpp` 1
-_ACEOF
- break
-fi
-done
-
-fi
-
ac_fn_c_check_func "$LINENO" "crypt" "ac_cv_func_crypt"
if test "x$ac_cv_func_crypt" = xyes; then :
$as_echo "#define HAVE_CRYPT 1" >>confdefs.h
diff --git a/configure.in b/configure.in
index 9398482..5c75fea 100644
--- a/configure.in
+++ b/configure.in
@@ -1502,25 +1502,6 @@ fi
AC_CHECK_DECLS([snprintf, vsnprintf])
-
-dnl Cannot use AC_CHECK_FUNC because isinf may be a macro
-AC_CACHE_CHECK([for isinf], ac_cv_func_isinf,
-[AC_LINK_IFELSE([AC_LANG_PROGRAM([
-#include <math.h>
-double glob_double;
-],
-[return isinf(glob_double) ? 0 : 1;])],
-[ac_cv_func_isinf=yes],
-[ac_cv_func_isinf=no])])
-
-if test $ac_cv_func_isinf = yes ; then
- AC_DEFINE(HAVE_ISINF, 1, [Define to 1 if you have isinf().])
-else
- AC_LIBOBJ(isinf)
- # Look for a way to implement a substitute for isinf()
- AC_CHECK_FUNCS([fpclass fp_class fp_class_d class], [break])
-fi
-
AC_REPLACE_FUNCS([crypt fls getopt getrusage inet_aton mkdtemp random rint srandom strerror strlcat strlcpy])
case $host_os in
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index 16a272e..deac60e 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -276,9 +276,6 @@
/* Define to 1 if you have support for IPv6. */
#undef HAVE_IPV6
-/* Define to 1 if you have isinf(). */
-#undef HAVE_ISINF
-
/* Define to 1 if you have the <langinfo.h> header file. */
#undef HAVE_LANGINFO_H
diff --git a/src/include/pg_config.h.win32 b/src/include/pg_config.h.win32
index 8566065..52cfbce 100644
--- a/src/include/pg_config.h.win32
+++ b/src/include/pg_config.h.win32
@@ -187,9 +187,6 @@
/* Define to 1 if you have support for IPv6. */
#define HAVE_IPV6 1
-/* Define to 1 if you have isinf(). */
-#define HAVE_ISINF 1
-
/* Define to 1 if you have the <langinfo.h> header file. */
/* #undef HAVE_LANGINFO_H */
diff --git a/src/include/port.h b/src/include/port.h
index 9fc79f4..e908340 100644
--- a/src/include/port.h
+++ b/src/include/port.h
@@ -374,10 +374,6 @@ extern int fls(int mask);
extern int getpeereid(int sock, uid_t *uid, gid_t *gid);
#endif
-#ifndef HAVE_ISINF
-extern int isinf(double x);
-#endif
-
#ifndef HAVE_MKDTEMP
extern char *mkdtemp(char *path);
#endif
diff --git a/src/interfaces/ecpg/ecpglib/.gitignore b/src/interfaces/ecpg/ecpglib/.gitignore
index 8ef6401..e23e8f0 100644
--- a/src/interfaces/ecpg/ecpglib/.gitignore
+++ b/src/interfaces/ecpg/ecpglib/.gitignore
@@ -7,4 +7,3 @@
/strlcpy.c
/thread.c
/win32setlocale.c
-/isinf.c
diff --git a/src/interfaces/ecpg/ecpglib/Makefile b/src/interfaces/ecpg/ecpglib/Makefile
index 39c4232..a27db67 100644
--- a/src/interfaces/ecpg/ecpglib/Makefile
+++ b/src/interfaces/ecpg/ecpglib/Makefile
@@ -27,7 +27,7 @@ LIBS := $(filter-out -lpgport, $(LIBS))
OBJS= execute.o typename.o descriptor.o sqlda.o data.o error.o prepare.o memory.o \
connect.o misc.o path.o pgstrcasecmp.o \
- $(filter snprintf.o strlcpy.o win32setlocale.o isinf.o, $(LIBOBJS)) $(WIN32RES)
+ $(filter snprintf.o strlcpy.o win32setlocale.o, $(LIBOBJS)) $(WIN32RES)
# thread.c is needed only for non-WIN32 implementation of path.c
ifneq ($(PORTNAME), win32)
@@ -55,7 +55,7 @@ include $(top_srcdir)/src/Makefile.shlib
# necessarily use the same object files as the backend uses. Instead,
# symlink the source files in here and build our own object file.
-path.c pgstrcasecmp.c snprintf.c strlcpy.c thread.c win32setlocale.c isinf.c: % : $(top_srcdir)/src/port/%
+path.c pgstrcasecmp.c snprintf.c strlcpy.c thread.c win32setlocale.c: % : $(top_srcdir)/src/port/%
rm -f $@ && $(LN_S) $< .
misc.o: misc.c $(top_builddir)/src/port/pg_config_paths.h
diff --git a/src/port/isinf.c b/src/port/isinf.c
deleted file mode 100644
index 7e8aabc..0000000
--- a/src/port/isinf.c
+++ /dev/null
@@ -1,77 +0,0 @@
-/*-------------------------------------------------------------------------
- *
- * isinf.c
- *
- * Portions Copyright (c) 1996-2016, PostgreSQL Global Development Group
- * Portions Copyright (c) 1994, Regents of the University of California
- *
- *
- * IDENTIFICATION
- * src/port/isinf.c
- *
- *-------------------------------------------------------------------------
- */
-
-#include "c.h"
-
-#include <float.h>
-#include <math.h>
-
-#if HAVE_FPCLASS /* this is _not_ HAVE_FP_CLASS, and not typo */
-
-#if HAVE_IEEEFP_H
-#include <ieeefp.h>
-#endif
-int
-isinf(double d)
-{
- fpclass_t type = fpclass(d);
-
- switch (type)
- {
- case FP_NINF:
- case FP_PINF:
- return 1;
- default:
- break;
- }
- return 0;
-}
-#else
-
-#if defined(HAVE_FP_CLASS) || defined(HAVE_FP_CLASS_D)
-
-#if HAVE_FP_CLASS_H
-#include <fp_class.h>
-#endif
-int
-isinf(x)
-double x;
-{
-#if HAVE_FP_CLASS
- int fpclass = fp_class(x);
-#else
- int fpclass = fp_class_d(x);
-#endif
-
- if (fpclass == FP_POS_INF)
- return 1;
- if (fpclass == FP_NEG_INF)
- return -1;
- return 0;
-}
-#elif defined(HAVE_CLASS)
-int
-isinf(double x)
-{
- int fpclass = class(x);
-
- if (fpclass == FP_PLUS_INF)
- return 1;
- if (fpclass == FP_MINUS_INF)
- return -1;
- return 0;
-}
-#endif
-
-#endif
Michael Paquier <michael.paquier@gmail.com> writes:
On Mon, Feb 1, 2016 at 2:38 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:Actually, is there actually a reason to keep this file in the code
tree? Are there platforms that do not have isinf()? Even for Windows
environments using MSVC < 1800 this is emulated using _fpclass.
Looking at what is in the buildfarm, I noticed that isinf is provided
everywhere. Attached is a patch. Thoughts?
Two comments:
1. I don't think the buildfarm is sufficient evidence to conclude that
isinf.c is required nowhere. It was in use as late as 2004, judging
by the git history, and I don't know of good reason to assume we do not
need it now.
2. POSIX:2008 only requires that "The isinf() macro shall return a
non-zero value if and only if its argument has an infinite value."
Therefore, any assumption about the sign of the result is wrong anyway,
and any patch that depends on it will be rejected, regardless of what
isinf.c does. Or else, if you can convince us that such an assumption
is really valuable, we'd need isinf.c more not less so that we can
replace isinf() on platforms where it doesn't meet the stronger spec.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Mon, Feb 1, 2016 at 4:04 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
1. I don't think the buildfarm is sufficient evidence to conclude that
isinf.c is required nowhere. It was in use as late as 2004, judging
by the git history, and I don't know of good reason to assume we do not
need it now.
This was 12 years ago... Btw, after a bit of digging, I found out that even
SunOS 5.1 includes it:
http://www.unix.com/man-page/sunos/3m/isinf/
--
Michael
On 1/31/16, Tom Lane <tgl@sss.pgh.pa.us> wrote:
2. POSIX:2008 only requires that "The isinf() macro shall return a
non-zero value if and only if its argument has an infinite value."
Therefore, any assumption about the sign of the result is wrong anyway,
and any patch that depends on it will be rejected, regardless of what
isinf.c does. Or else, if you can convince us that such an assumption
is really valuable, we'd need isinf.c more not less so that we can
replace isinf() on platforms where it doesn't meet the stronger spec.regards, tom lane
Ok, then I'll use "is_infinite" from "float.c".
But why functions' (in "src/port/isinf.c") behavior are different? It
is a bit confusing…
--
Best regards,
Vitaly Burovoy
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Vitaly Burovoy <vitaly.burovoy@gmail.com> writes:
On 1/31/16, Tom Lane <tgl@sss.pgh.pa.us> wrote:
2. POSIX:2008 only requires that "The isinf() macro shall return a
non-zero value if and only if its argument has an infinite value."
Ok, then I'll use "is_infinite" from "float.c".
Yeah, that's good.
But why functions' (in "src/port/isinf.c") behavior are different? It
is a bit confusing…
Probably the authors of those different implementations were making it
match the behavior of whatever isinf() they had locally. I don't feel
a need to change isinf.c --- it should be zero-maintenance at this
point, especially if we suspect it is no longer used anywhere.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers