Re: [Emacs-diffs] master cc5325b: Pacify -Wcast-function-type warnings in GCC 8.1

Re: [Emacs-diffs] master cc5325b: Pacify -Wcast-function-type warnings in GCC 8.1

[Ken Brown]

On 8/17/2018 10:31 AM, Eli Zaretskii wrote:
> branch: master
> commit cc5325b0bea13bd93478fcee0b035877b3a72290
> Author: Andy Moreton <andrewjmoreton@gmail.com>
> Commit: Eli Zaretskii <eliz@gnu.org>
> 
>      Pacify -Wcast-function-type warnings in GCC 8.1
>      
>      * src/w32.h: (get_proc_addr): New function.
>      * src/w32fns.c (setup_w32_kbdhook, Ffile_system_info)
>      * src/w32uniscribe.c (syms_of_w32uniscribe): Use get_proc_addr.

This breaks the Cygwin-w32 build because w32fns.c and w32uniscribe.c 
don't include w32.h on Cygwin, so get_proc_addr is undefined.  Could the 
definition of get_proc_addr be moved to a different header that gets 
included in the Cygwin build?  Maybe w32common.h?

Ken

Re: [Emacs-diffs] master cc5325b: Pacify -Wcast-function-type warnings in GCC 8.1

[Andy Moreton]

[-- Attachment #1: Type: text/plain, Size: 896 bytes --]

On Mon 20 Aug 2018, Ken Brown wrote:

> On 8/17/2018 10:31 AM, Eli Zaretskii wrote:
>> branch: master
>> commit cc5325b0bea13bd93478fcee0b035877b3a72290
>> Author: Andy Moreton <andrewjmoreton@gmail.com>
>> Commit: Eli Zaretskii <eliz@gnu.org>
>>
>>      Pacify -Wcast-function-type warnings in GCC 8.1
>>           * src/w32.h: (get_proc_addr): New function.
>>      * src/w32fns.c (setup_w32_kbdhook, Ffile_system_info)
>>      * src/w32uniscribe.c (syms_of_w32uniscribe): Use get_proc_addr.
>
> This breaks the Cygwin-w32 build because w32fns.c and w32uniscribe.c don't
> include w32.h on Cygwin, so get_proc_addr is undefined.  Could the definition
> of get_proc_addr be moved to a different header that gets included in the
> Cygwin build?  Maybe w32common.h?

Sorry Ken, I forgot to test this Cygwin as well as mingw32 and mingw64.
Does something like this patch work for you ?

    AndyM


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: cygwin-fix-get-proc-addr.patch --]
[-- Type: text/x-patch, Size: 5703 bytes --]

diff --git a/src/decompress.c b/src/decompress.c
index 9f7fbe4195..2836338216 100644
--- a/src/decompress.c
+++ b/src/decompress.c
@@ -30,6 +30,7 @@ along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.  */
 
 #ifdef WINDOWSNT
 # include <windows.h>
+# include "w32common.h"
 # include "w32.h"
 
 DEF_DLL_FN (int, inflateInit2_,
diff --git a/src/gnutls.c b/src/gnutls.c
index 2d455ea1ba..aa5c97532f 100644
--- a/src/gnutls.c
+++ b/src/gnutls.c
@@ -58,6 +58,7 @@ along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.  */
 
 # ifdef WINDOWSNT
 #  include <windows.h>
+#  include "w32common.h"
 #  include "w32.h"
 # endif
 
diff --git a/src/image.c b/src/image.c
index 7866b9cc46..b9ff3f25c4 100644
--- a/src/image.c
+++ b/src/image.c
@@ -77,6 +77,7 @@ typedef struct x_bitmap_record Bitmap_Record;
 
 /* We need (or want) w32.h only when we're _not_ compiling for Cygwin.  */
 #ifdef WINDOWSNT
+# include "w32common.h"
 # include "w32.h"
 #endif
 
diff --git a/src/json.c b/src/json.c
index 4e413a2bd5..4e46640a0c 100644
--- a/src/json.c
+++ b/src/json.c
@@ -34,6 +34,7 @@ along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.  */
 
 #ifdef WINDOWSNT
 # include <windows.h>
+# include "w32common.h"
 # include "w32.h"
 
 DEF_DLL_FN (void, json_set_alloc_funcs,
diff --git a/src/lcms.c b/src/lcms.c
index a3a9822306..f37f843e50 100644
--- a/src/lcms.c
+++ b/src/lcms.c
@@ -34,6 +34,7 @@ typedef struct
 
 #ifdef WINDOWSNT
 # include <windows.h>
+# include "w32common.h"
 # include "w32.h"
 
 DEF_DLL_FN (cmsFloat64Number, cmsCIE2000DeltaE,
diff --git a/src/w32.h b/src/w32.h
index a053ee0fc4..9c219cdda6 100644
--- a/src/w32.h
+++ b/src/w32.h
@@ -164,10 +164,6 @@ extern void reset_standard_handles (int in, int out,
 /* Return the string resource associated with KEY of type TYPE.  */
 extern LPBYTE w32_get_resource (const char * key, LPDWORD type);
 
-/* Load a function from a DLL.  Defined in this file.  */
-typedef void (* VOIDFNPTR) (void);
-INLINE VOIDFNPTR get_proc_addr (HINSTANCE handle, LPCSTR fname);
-
 extern void release_listen_threads (void);
 extern void init_ntproc (int);
 extern void term_ntproc (int);
@@ -245,33 +241,4 @@ extern ssize_t emacs_gnutls_push (gnutls_transport_ptr_t p,
                                   const void* buf, size_t sz);
 #endif /* HAVE_GNUTLS */
 
-
-
-/* Load a function address from a DLL. Cast the result via "VOIDFNPTR"
-   to pacify -Wcast-function-type in GCC 8.1.  */
-INLINE VOIDFNPTR
-get_proc_addr (HINSTANCE handle, LPCSTR fname)
-{
-  return (VOIDFNPTR) GetProcAddress (handle, fname);
-}
-
-/* Define a function that will be loaded from a DLL.  The variable
-   arguments should contain the argument list for the function, and
-   optionally be followed by function attributes.  For example:
-   DEF_DLL_FN (void, png_longjmp, (png_structp, int) PNG_NORETURN);
-  */
-#define DEF_DLL_FN(type, func, ...)                     \
-  typedef type (CDECL *W32_PFN_##func) __VA_ARGS__;     \
-  static W32_PFN_##func fn_##func
-
-/* Load a function from the DLL.  */
-#define LOAD_DLL_FN(lib, func)						\
-  do									\
-    {									\
-      fn_##func = (W32_PFN_##func) get_proc_addr (lib, #func);   \
-      if (!fn_##func)							\
-	return false;							\
-    }									\
-  while (false)
-
 #endif /* EMACS_W32_H */
diff --git a/src/w32common.h b/src/w32common.h
index af548dd8ea..4981bdfd89 100644
--- a/src/w32common.h
+++ b/src/w32common.h
@@ -50,4 +50,34 @@ extern int os_subtype;
 /* Cache system info, e.g., the NT page size.  */
 extern void cache_system_info (void);
 
+typedef void (* VOIDFNPTR) (void);
+
+/* Load a function address from a DLL.  Cast the result via VOIDFNPTR
+   to pacify -Wcast-function-type in GCC 8.1.  The return value must
+   be cast to the correct function pointer type.  */
+INLINE VOIDFNPTR
+get_proc_addr (HINSTANCE handle, LPCSTR fname)
+{
+  return (VOIDFNPTR) GetProcAddress (handle, fname);
+}
+
+/* Define a function that will be loaded from a DLL.  The variable
+   arguments should contain the argument list for the function, and
+   optionally be followed by function attributes.  For example:
+   DEF_DLL_FN (void, png_longjmp, (png_structp, int) PNG_NORETURN);
+  */
+#define DEF_DLL_FN(type, func, ...)                     \
+  typedef type (CDECL *W32_PFN_##func) __VA_ARGS__;     \
+  static W32_PFN_##func fn_##func
+
+/* Load a function from the DLL.  */
+#define LOAD_DLL_FN(lib, func)						\
+  do									\
+    {									\
+      fn_##func = (W32_PFN_##func) get_proc_addr (lib, #func);		\
+      if (!fn_##func)							\
+	return false;							\
+    }									\
+  while (false)
+
 #endif /* W32COMMON_H */
diff --git a/src/w32font.c b/src/w32font.c
index f613061832..798869b5ca 100644
--- a/src/w32font.c
+++ b/src/w32font.c
@@ -29,6 +29,7 @@ along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.  */
 #include "coding.h"	/* for ENCODE_SYSTEM, DECODE_SYSTEM */
 #include "w32font.h"
 #ifdef WINDOWSNT
+#include "w32common.h"
 #include "w32.h"
 #endif
 
diff --git a/src/w32uniscribe.c b/src/w32uniscribe.c
index 54f161690b..29c9c7a0bd 100644
--- a/src/w32uniscribe.c
+++ b/src/w32uniscribe.c
@@ -36,6 +36,7 @@ along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.  */
 #include "composite.h"
 #include "font.h"
 #include "w32font.h"
+#include "w32common.h"
 
 struct uniscribe_font_info
 {
diff --git a/src/xml.c b/src/xml.c
index 5f3ccc85c8..e85891d2a2 100644
--- a/src/xml.c
+++ b/src/xml.c
@@ -31,6 +31,7 @@ along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.  */
 #ifdef WINDOWSNT
 
 # include <windows.h>
+# include "w32common.h"
 # include "w32.h"
 
 DEF_DLL_FN (htmlDocPtr, htmlReadMemory,

Re: [Emacs-diffs] master cc5325b: Pacify -Wcast-function-type warnings in GCC 8.1

[Ken Brown]

On 8/20/2018 10:18 AM, Andy Moreton wrote:
> On Mon 20 Aug 2018, Ken Brown wrote:
> 
>> On 8/17/2018 10:31 AM, Eli Zaretskii wrote:
>>> branch: master
>>> commit cc5325b0bea13bd93478fcee0b035877b3a72290
>>> Author: Andy Moreton <andrewjmoreton@gmail.com>
>>> Commit: Eli Zaretskii <eliz@gnu.org>
>>>
>>>       Pacify -Wcast-function-type warnings in GCC 8.1
>>>            * src/w32.h: (get_proc_addr): New function.
>>>       * src/w32fns.c (setup_w32_kbdhook, Ffile_system_info)
>>>       * src/w32uniscribe.c (syms_of_w32uniscribe): Use get_proc_addr.
>>
>> This breaks the Cygwin-w32 build because w32fns.c and w32uniscribe.c don't
>> include w32.h on Cygwin, so get_proc_addr is undefined.  Could the definition
>> of get_proc_addr be moved to a different header that gets included in the
>> Cygwin build?  Maybe w32common.h?
> 
> Sorry Ken, I forgot to test this Cygwin as well as mingw32 and mingw64.
> Does something like this patch work for you ?

Yes, that fixes it.

Thanks.

Ken

Re: [Emacs-diffs] master cc5325b: Pacify -Wcast-function-type warnings in GCC 8.1

[Eli Zaretskii]

> From: Ken Brown <kbrown@cornell.edu>
> Date: Mon, 20 Aug 2018 09:29:03 -0400
> 
> This breaks the Cygwin-w32 build because w32fns.c and w32uniscribe.c 
> don't include w32.h on Cygwin, so get_proc_addr is undefined.

Sorry about that.

> Could the definition of get_proc_addr be moved to a different header
> that gets included in the Cygwin build?  Maybe w32common.h?

Moving to w32common.h is fine, but w32font.c doesn't include it.  So
something else is necessary.

Re: [Emacs-diffs] master cc5325b: Pacify -Wcast-function-type warnings in GCC 8.1

[Ken Brown]

On 8/20/2018 10:45 AM, Ken Brown wrote:
> On 8/20/2018 10:18 AM, Andy Moreton wrote:
>> Sorry Ken, I forgot to test this Cygwin as well as mingw32 and mingw64.
>> Does something like this patch work for you ?
> 
> Yes, that fixes it.

I went ahead and committed this in your name.

Ken