From: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Andres Freund <andres@anarazel.de>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: warnings for invalid function casts
Date: Sat, 4 Jul 2020 13:36:44 +0200
Message-ID: <d04f3b48-8195-aa83-8862-b6d3747fed30@2ndquadrant.com> (raw)
In-Reply-To: <931660.1593787200@sss.pgh.pa.us>
References: <1e97628e-6447-b4fd-e230-d109cec2d584@2ndquadrant.com>
<474790.1593526505@sss.pgh.pa.us>
<20200630192135.nwlza2u57dy3qej4@alap3.anarazel.de>
<496552.1593545900@sss.pgh.pa.us>
<c764383a-3f9e-f5ba-79d1-8824fe73340f@2ndquadrant.com>
<931660.1593787200@sss.pgh.pa.us>
On 2020-07-03 16:40, Tom Lane wrote:
> Given that gcc explicitly documents "void (*) (void)" as being what
> to use, they're going to have a hard time changing their minds about
> that ... and gcc is dominant enough in this space that I suppose
> other compilers would have to be compatible with it. So even though
> it's theoretically bogus, I suppose we might as well go along with
> it. The typedef will allow a centralized fix if we ever find a
> better answer.
Do people prefer a typedef or just writing it out, like it's done in the
Python code?
Attached is a provisional patch that has it written out.
I'm minimally in favor of that, since the Python code would be
consistent with the Python core code, and the one other use is quite
special and it might not be worth introducing a globally visible
workaround for it. But if we prefer a typedef then I'd propose
GenericFuncPtr like in the initial patch.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:v2-0001-Fix-Wcast-function-type-warnings.patch (text/plain, 9.5K) download
From 220bab24e5a58f1b5d66336427c0877329a5f88a Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Sat, 4 Jul 2020 13:15:12 +0200
Subject: [PATCH v2] Fix -Wcast-function-type warnings
Discussion: https://www.postgresql.org/message-id/flat/20180206200205.f5kvbyn6jawtzi6s%40alap3.anarazel.de
---
configure | 91 +++++++++++++++++++++++++++++++
configure.in | 2 +
src/backend/utils/fmgr/dfmgr.c | 14 ++---
src/backend/utils/hash/dynahash.c | 9 ++-
src/include/fmgr.h | 6 +-
src/pl/plpython/plpy_plpymodule.c | 14 ++---
6 files changed, 118 insertions(+), 18 deletions(-)
diff --git a/configure b/configureindex 2feff37fe3..9907637e31 100755--- a/configure+++ b/configure@@ -5643,6 +5643,97 @@ if test x"$pgac_cv_prog_CXX_cxxflags__Wimplicit_fallthrough_3" = x"yes"; then
fi
++{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${CC} supports -Wcast-function-type, for CFLAGS" >&5+$as_echo_n "checking whether ${CC} supports -Wcast-function-type, for CFLAGS... " >&6; }+if ${pgac_cv_prog_CC_cflags__Wcast_function_type+:} false; then :+ $as_echo_n "(cached) " >&6+else+ pgac_save_CFLAGS=$CFLAGS+pgac_save_CC=$CC+CC=${CC}+CFLAGS="${CFLAGS} -Wcast-function-type"+ac_save_c_werror_flag=$ac_c_werror_flag+ac_c_werror_flag=yes+cat confdefs.h - <<_ACEOF >conftest.$ac_ext+/* end confdefs.h. */++int+main ()+{++ ;+ return 0;+}+_ACEOF+if ac_fn_c_try_compile "$LINENO"; then :+ pgac_cv_prog_CC_cflags__Wcast_function_type=yes+else+ pgac_cv_prog_CC_cflags__Wcast_function_type=no+fi+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext+ac_c_werror_flag=$ac_save_c_werror_flag+CFLAGS="$pgac_save_CFLAGS"+CC="$pgac_save_CC"+fi+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_prog_CC_cflags__Wcast_function_type" >&5+$as_echo "$pgac_cv_prog_CC_cflags__Wcast_function_type" >&6; }+if test x"$pgac_cv_prog_CC_cflags__Wcast_function_type" = x"yes"; then+ CFLAGS="${CFLAGS} -Wcast-function-type"+fi+++ { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${CXX} supports -Wcast-function-type, for CXXFLAGS" >&5+$as_echo_n "checking whether ${CXX} supports -Wcast-function-type, for CXXFLAGS... " >&6; }+if ${pgac_cv_prog_CXX_cxxflags__Wcast_function_type+:} false; then :+ $as_echo_n "(cached) " >&6+else+ pgac_save_CXXFLAGS=$CXXFLAGS+pgac_save_CXX=$CXX+CXX=${CXX}+CXXFLAGS="${CXXFLAGS} -Wcast-function-type"+ac_save_cxx_werror_flag=$ac_cxx_werror_flag+ac_cxx_werror_flag=yes+ac_ext=cpp+ac_cpp='$CXXCPP $CPPFLAGS'+ac_compile='$CXX -c $CXXFLAGS $CPPFLAGS conftest.$ac_ext >&5'+ac_link='$CXX -o conftest$ac_exeext $CXXFLAGS $CPPFLAGS $LDFLAGS conftest.$ac_ext $LIBS >&5'+ac_compiler_gnu=$ac_cv_cxx_compiler_gnu++cat confdefs.h - <<_ACEOF >conftest.$ac_ext+/* end confdefs.h. */++int+main ()+{++ ;+ return 0;+}+_ACEOF+if ac_fn_cxx_try_compile "$LINENO"; then :+ pgac_cv_prog_CXX_cxxflags__Wcast_function_type=yes+else+ pgac_cv_prog_CXX_cxxflags__Wcast_function_type=no+fi+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext+ac_ext=c+ac_cpp='$CPP $CPPFLAGS'+ac_compile='$CC -c $CFLAGS $CPPFLAGS conftest.$ac_ext >&5'+ac_link='$CC -o conftest$ac_exeext $CFLAGS $CPPFLAGS $LDFLAGS conftest.$ac_ext $LIBS >&5'+ac_compiler_gnu=$ac_cv_c_compiler_gnu++ac_cxx_werror_flag=$ac_save_cxx_werror_flag+CXXFLAGS="$pgac_save_CXXFLAGS"+CXX="$pgac_save_CXX"+fi+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_prog_CXX_cxxflags__Wcast_function_type" >&5+$as_echo "$pgac_cv_prog_CXX_cxxflags__Wcast_function_type" >&6; }+if test x"$pgac_cv_prog_CXX_cxxflags__Wcast_function_type" = x"yes"; then+ CXXFLAGS="${CXXFLAGS} -Wcast-function-type"+fi++
# This was included in -Wall/-Wformat in older GCC versions
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${CC} supports -Wformat-security, for CFLAGS" >&5
diff --git a/configure.in b/configure.inindex 0188c6ff07..2e05ce2e4d 100644--- a/configure.in+++ b/configure.in@@ -498,6 +498,8 @@ if test "$GCC" = yes -a "$ICC" = no; then
PGAC_PROG_CXX_CFLAGS_OPT([-Wmissing-format-attribute])
PGAC_PROG_CC_CFLAGS_OPT([-Wimplicit-fallthrough=3])
PGAC_PROG_CXX_CFLAGS_OPT([-Wimplicit-fallthrough=3])
+ PGAC_PROG_CC_CFLAGS_OPT([-Wcast-function-type])+ PGAC_PROG_CXX_CFLAGS_OPT([-Wcast-function-type])
# This was included in -Wall/-Wformat in older GCC versions
PGAC_PROG_CC_CFLAGS_OPT([-Wformat-security])
PGAC_PROG_CXX_CFLAGS_OPT([-Wformat-security])
diff --git a/src/backend/utils/fmgr/dfmgr.c b/src/backend/utils/fmgr/dfmgr.cindex 9dff1f5e82..bd779fdaf7 100644--- a/src/backend/utils/fmgr/dfmgr.c+++ b/src/backend/utils/fmgr/dfmgr.c@@ -95,7 +95,7 @@ static const Pg_magic_struct magic_data = PG_MODULE_MAGIC_DATA;
* named funcname in it.
*
* If the function is not found, we raise an error if signalNotFound is true,
- * else return (PGFunction) NULL. Note that errors in loading the library+ * else return NULL. Note that errors in loading the library
* will provoke ereport() regardless of signalNotFound.
*
* If filehandle is not NULL, then *filehandle will be set to a handle
@@ -103,13 +103,13 @@ static const Pg_magic_struct magic_data = PG_MODULE_MAGIC_DATA;
* lookup_external_function to lookup additional functions in the same file
* at less cost than repeating load_external_function.
*/
-PGFunction+void *
load_external_function(const char *filename, const char *funcname,
bool signalNotFound, void **filehandle)
{
char *fullname;
void *lib_handle;
- PGFunction retval;+ void *retval;
/* Expand the possibly-abbreviated filename to an exact path name */
fullname = expand_dynamic_library_name(filename);
@@ -122,7 +122,7 @@ load_external_function(const char *filename, const char *funcname,
*filehandle = lib_handle;
/* Look up the function within the library. */
- retval = (PGFunction) dlsym(lib_handle, funcname);+ retval = dlsym(lib_handle, funcname);
if (retval == NULL && signalNotFound)
ereport(ERROR,
@@ -165,12 +165,12 @@ load_file(const char *filename, bool restricted)
/*
* Lookup a function whose library file is already loaded.
- * Return (PGFunction) NULL if not found.+ * Return NULL if not found.
*/
-PGFunction+void *
lookup_external_function(void *filehandle, const char *funcname)
{
- return (PGFunction) dlsym(filehandle, funcname);+ return dlsym(filehandle, funcname);
}
diff --git a/src/backend/utils/hash/dynahash.c b/src/backend/utils/hash/dynahash.cindex 2688e27726..7c8039d0ef 100644--- a/src/backend/utils/hash/dynahash.c+++ b/src/backend/utils/hash/dynahash.c@@ -398,7 +398,14 @@ hash_create(const char *tabname, long nelem, HASHCTL *info, int flags)
if (flags & HASH_KEYCOPY)
hashp->keycopy = info->keycopy;
else if (hashp->hash == string_hash)
- hashp->keycopy = (HashCopyFunc) strlcpy;+ /*+ * The signature of keycopy is meant for memcpy(), which returns+ * void*, but strlcpy() returns size_t. Since we never use the return+ * value of keycopy, and size_t is pretty much always the same size as+ * void *, this should be safe. The extra cast in the middle is to+ * avoid warnings from -Wcast-function-type.+ */+ hashp->keycopy = (HashCopyFunc) (void (*) (void)) strlcpy;
else
hashp->keycopy = memcpy;
diff --git a/src/include/fmgr.h b/src/include/fmgr.hindex d349510b7c..f25068fae2 100644--- a/src/include/fmgr.h+++ b/src/include/fmgr.h@@ -716,9 +716,9 @@ extern bool CheckFunctionValidatorAccess(Oid validatorOid, Oid functionOid);
*/
extern char *Dynamic_library_path;
-extern PGFunction load_external_function(const char *filename, const char *funcname,- bool signalNotFound, void **filehandle);-extern PGFunction lookup_external_function(void *filehandle, const char *funcname);+extern void *load_external_function(const char *filename, const char *funcname,+ bool signalNotFound, void **filehandle);+extern void *lookup_external_function(void *filehandle, const char *funcname);
extern void load_file(const char *filename, bool restricted);
extern void **find_rendezvous_variable(const char *varName);
extern Size EstimateLibraryStateSpace(void);
diff --git a/src/pl/plpython/plpy_plpymodule.c b/src/pl/plpython/plpy_plpymodule.cindex e308c61d50..6356afbbd4 100644--- a/src/pl/plpython/plpy_plpymodule.c+++ b/src/pl/plpython/plpy_plpymodule.c@@ -61,13 +61,13 @@ static PyMethodDef PLy_methods[] = {
/*
* logging methods
*/
- {"debug", (PyCFunction) PLy_debug, METH_VARARGS | METH_KEYWORDS, NULL},- {"log", (PyCFunction) PLy_log, METH_VARARGS | METH_KEYWORDS, NULL},- {"info", (PyCFunction) PLy_info, METH_VARARGS | METH_KEYWORDS, NULL},- {"notice", (PyCFunction) PLy_notice, METH_VARARGS | METH_KEYWORDS, NULL},- {"warning", (PyCFunction) PLy_warning, METH_VARARGS | METH_KEYWORDS, NULL},- {"error", (PyCFunction) PLy_error, METH_VARARGS | METH_KEYWORDS, NULL},- {"fatal", (PyCFunction) PLy_fatal, METH_VARARGS | METH_KEYWORDS, NULL},+ {"debug", (PyCFunction) (void (*) (void)) PLy_debug, METH_VARARGS | METH_KEYWORDS, NULL},+ {"log", (PyCFunction) (void (*) (void)) PLy_log, METH_VARARGS | METH_KEYWORDS, NULL},+ {"info", (PyCFunction) (void (*) (void)) PLy_info, METH_VARARGS | METH_KEYWORDS, NULL},+ {"notice", (PyCFunction) (void (*) (void)) PLy_notice, METH_VARARGS | METH_KEYWORDS, NULL},+ {"warning", (PyCFunction) (void (*) (void)) PLy_warning, METH_VARARGS | METH_KEYWORDS, NULL},+ {"error", (PyCFunction) (void (*) (void)) PLy_error, METH_VARARGS | METH_KEYWORDS, NULL},+ {"fatal", (PyCFunction) (void (*) (void)) PLy_fatal, METH_VARARGS | METH_KEYWORDS, NULL},
/*
* create a stored plan
--
2.27.0
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: peter.eisentraut@2ndquadrant.com, tgl@sss.pgh.pa.us, andres@anarazel.de
Subject: Re: warnings for invalid function casts
In-Reply-To: <d04f3b48-8195-aa83-8862-b6d3747fed30@2ndquadrant.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox