Fix -Wcast-function-type warnings

Three groups of issues needed to be addressed:

load_external_function() and related functions returned PGFunction,
even though not necessarily all callers are looking for a function of
type PGFunction.  Since these functions are really just wrappers
around dlsym(), change to return void * just like dlsym().

In dynahash.c, we are using strlcpy() where a function with a
signature like memcpy() is expected.  This should be safe, as the new
comment there explains, but the cast needs to be augmented to avoid
the warning.

In PL/Python, methods all need to be cast to PyCFunction, per Python
API, but this now runs afoul of these warnings.  (This issue also
exists in core CPython.)

To fix the second and third case, we add a new type pg_funcptr_t that
is defined specifically so that gcc accepts it as a special function
pointer that can be cast to any other function pointer without the
warning.

Also add -Wcast-function-type to the standard warning flags, subject
to configure check.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/1e97628e-6447-b4fd-e230-d109cec2d584%402ndquadrant.com

diff --git a/configure b/configure
index 2feff37fe371dcc9b7ac1542768f88336c6fca78..9907637e3176ed9a56f541d8c3be65be8ebb8f3e 100755 (executable)
--- 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.in
index 0188c6ff074e605a15c120ac40e3c4f41152fbd1..2e05ce2e4d69915dcdbe6b9da9e6f2439f7a2510 100644 (file)
--- 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.c
index 9dff1f5e8291a6ff49d40295c70b538be8ac8017..bd779fdaf7af581b1b900f64db4ed69ff9972f8a 100644 (file)
--- 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.c
index 2688e277267e26d29b4149c3e6f5cbf790ac6365..5948b01abc34e770ef7ae57d09224af93b3d0118 100644 (file)
--- a/src/backend/utils/hash/dynahash.c
+++ b/src/backend/utils/hash/dynahash.c
@@ -398,7 +398,16 @@ 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) (pg_funcptr_t) strlcpy;
+   }
    else
        hashp->keycopy = memcpy;
 

diff --git a/src/include/c.h b/src/include/c.h
index a904b49a37fd5f84d08177098d5e16793d0787fe..f242e32edbe707f473746c0da8e46027a9f9144e 100644 (file)
--- a/src/include/c.h
+++ b/src/include/c.h
@@ -265,6 +265,13 @@
 #define dummyret   char
 #endif
 
+/*
+ * Generic function pointer.  This can be used in the rare cases where it's
+ * necessary to cast a function pointer to a seemingly incompatible function
+ * pointer type while avoiding gcc's -Wcast-function-type warnings.
+ */
+typedef void (*pg_funcptr_t) (void);
+
 /*
  * We require C99, hence the compiler should understand flexible array
  * members.  However, for documentation purposes we still consider it to be

diff --git a/src/include/fmgr.h b/src/include/fmgr.h
index d349510b7c76cb95cabb0623a01182929d556003..f25068fae201ac420360400f1a07309e2346f0c8 100644 (file)
--- 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.c
index e308c61d50fe522352e908baf607c9ff9a08c7f1..7f54d093ace62129238895a2d6042eb17f9463ad 100644 (file)
--- 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) (pg_funcptr_t) PLy_debug, METH_VARARGS | METH_KEYWORDS, NULL},
+   {"log", (PyCFunction) (pg_funcptr_t) PLy_log, METH_VARARGS | METH_KEYWORDS, NULL},
+   {"info", (PyCFunction) (pg_funcptr_t) PLy_info, METH_VARARGS | METH_KEYWORDS, NULL},
+   {"notice", (PyCFunction) (pg_funcptr_t) PLy_notice, METH_VARARGS | METH_KEYWORDS, NULL},
+   {"warning", (PyCFunction) (pg_funcptr_t) PLy_warning, METH_VARARGS | METH_KEYWORDS, NULL},
+   {"error", (PyCFunction) (pg_funcptr_t) PLy_error, METH_VARARGS | METH_KEYWORDS, NULL},
+   {"fatal", (PyCFunction) (pg_funcptr_t) PLy_fatal, METH_VARARGS | METH_KEYWORDS, NULL},
 
    /*
     * create a stored plan