Re: [PATCH] Clean up a couple of compiler warnings

[Paul Eggert <eggert@cs.ucla.edu>]

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

Philipp Stephani wrote:

 > * sysdep.c (system_process_attributes): Remove unused variables.

Thanks, I installed that in your name (1st attached patch).


> * emacs.c (using_utf8): Don't assume anything about mbstate_t type.

The old code doesn't assume anything about the type either, and should work 
regardless of how mbstate_t is implemented. The proposed change makes the code a 
bit harder to read, so I'd rather avoid it if possible. I couldn't reproduce the 
warning that you're evidently seeing. Perhaps you can work around the problem in 
the same way I worked around the problem with floats and doubles (see below)?


> * emacs-module.c (MODULE_SETJMP_1): Mark dummy variable as unused.

Rather than do that, let's just use the variable; that's more robust. I 
installed the 2nd attached patch.

> * lread.c (string_to_number): Use constants of double type.
> * editfns.c (decode_float_time):
> * fns.c (make_hash_table, maybe_resize_hash_table)
> (Fhash_table_rehash_size, Fhash_table_rehash_threshold):
> Explicitly cast floating-point values.

There should be no problem (i.e., no loss of info) converting a float to a 
double, so these changes are not needed. I assume you're using clang. I 
reproduced its false alarms on Fedora 25 x86-64, which uses clang 3.9.1, and 
installed the 3rd attached patch to work around the problem.


 > * fileio.c (file_name_case_insensitive_p): Add cast.

Rather than waste time static-checking the DARWIN_OS_CASE_SENSITIVE_FIXME == 2 
code let's just #ifdef it out. I did that in the 4th attached patch. Maybe we 
should just remove it, since nobody is using it and (as you note) it doesn't 
work anyway.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Clean-up-some-compiler-warnings.patch --]
[-- Type: text/x-patch; name="0001-Clean-up-some-compiler-warnings.patch", Size: 854 bytes --]

From e41030971f37375b9bb16c248f3b5d8d12064743 Mon Sep 17 00:00:00 2001
From: Philipp Stephani <phst@google.com>
Date: Thu, 18 May 2017 19:15:26 -0700
Subject: [PATCH 1/4] Clean up some compiler warnings

* src/sysdep.c (system_process_attributes) [DARWIN_OS]:
Remove unused locals.
---
 src/sysdep.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/src/sysdep.c b/src/sysdep.c
index 91b2a5c..ac6eed0 100644
--- a/src/sysdep.c
+++ b/src/sysdep.c
@@ -3707,14 +3707,9 @@ Lisp_Object
 system_process_attributes (Lisp_Object pid)
 {
   int proc_id;
-  int pagesize = getpagesize ();
-  unsigned long npages;
-  int fscale;
   struct passwd *pw;
   struct group  *gr;
   char *ttyname;
-  size_t len;
-  char args[MAXPATHLEN];
   struct timeval starttime;
   struct timespec t, now;
   struct rusage *rusage;
-- 
2.7.4


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Clean-up-compiler-warning-in-emacs-module.c.patch --]
[-- Type: text/x-patch; name="0002-Clean-up-compiler-warning-in-emacs-module.c.patch", Size: 2626 bytes --]

From 9647430ae8744bd4dee29f1269b74f65c09274d2 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Thu, 18 May 2017 20:02:42 -0700
Subject: [PATCH 2/4] Clean up compiler warning in emacs-module.c

* src/emacs-module.c (MODULE_SETJMP_1): Use the local var
instead of leaving it unused, to pacify picky compilers.
(module_reset_handlerlist): Now takes a dummy pointer to a struct
handler *, instead of a dummy pointer to an int.  All uses changed.
---
 src/emacs-module.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/src/emacs-module.c b/src/emacs-module.c
index cd025a1..5aa8a88 100644
--- a/src/emacs-module.c
+++ b/src/emacs-module.c
@@ -109,7 +109,7 @@ static void module_handle_throw (emacs_env *, Lisp_Object);
 static void module_non_local_exit_signal_1 (emacs_env *, Lisp_Object, Lisp_Object);
 static void module_non_local_exit_throw_1 (emacs_env *, Lisp_Object, Lisp_Object);
 static void module_out_of_memory (emacs_env *);
-static void module_reset_handlerlist (const int *);
+static void module_reset_handlerlist (struct handler *const *);
 
 /* We used to return NULL when emacs_value was a different type from
    Lisp_Object, but nowadays we just use Qnil instead.  Although they
@@ -160,17 +160,18 @@ static emacs_value const module_nil = 0;
 
 /* TODO: Make backtraces work if this macros is used.  */
 
-#define MODULE_SETJMP_1(handlertype, handlerfunc, retval, c, dummy)	\
+#define MODULE_SETJMP_1(handlertype, handlerfunc, retval, c0, c)	\
   if (module_non_local_exit_check (env) != emacs_funcall_exit_return)	\
     return retval;							\
-  struct handler *c = push_handler_nosignal (Qt, handlertype);		\
-  if (!c)								\
+  struct handler *c0 = push_handler_nosignal (Qt, handlertype);		\
+  if (!c0)								\
     {									\
       module_out_of_memory (env);					\
       return retval;							\
     }									\
   verify (module_has_cleanup);						\
-  int dummy __attribute__ ((cleanup (module_reset_handlerlist)));	\
+  struct handler *c __attribute__ ((cleanup (module_reset_handlerlist))) \
+    = c0;								\
   if (sys_setjmp (c->jmp))						\
     {									\
       (handlerfunc) (env, c->val);					\
@@ -919,7 +920,7 @@ finalize_environment (struct emacs_env_private *env)
    code in eval.c for details.  The macros below arrange for this
    function to be called automatically.  DUMMY is ignored.  */
 static void
-module_reset_handlerlist (const int *dummy)
+module_reset_handlerlist (struct handler *const *dummy)
 {
   handlerlist = handlerlist->next;
 }
-- 
2.7.4


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0003-Port-enable-gcc-warnings-to-clang-3.9.1.patch --]
[-- Type: text/x-patch; name="0003-Port-enable-gcc-warnings-to-clang-3.9.1.patch", Size: 780 bytes --]

From af4ac3dc852647e5ea41be5ffc387be591978aee Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Thu, 18 May 2017 20:27:11 -0700
Subject: [PATCH 3/4] Port --enable-gcc-warnings to clang 3.9.1

* configure.ac (WERROR_CFLAGS): Omit -Wdouble-promotion if clang.
Problem reported by Philipp Stephani in:
http://lists.gnu.org/archive/html/emacs-devel/2017-05/msg00495.html
---
 configure.ac | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configure.ac b/configure.ac
index 34b75a7..03542a6 100644
--- a/configure.ac
+++ b/configure.ac
@@ -963,6 +963,7 @@ AC_DEFUN
 
   if test $emacs_cv_clang = yes; then
     nw="$nw -Wcast-align"
+    nw="$nw -Wdouble-promotion"
   fi
 
   # This causes too much noise in the MinGW build
-- 
2.7.4


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #5: 0004-Fix-DARWIN_OS_CASE_SENSITIVE_FIXME-2-false-alarm.patch --]
[-- Type: text/x-patch; name="0004-Fix-DARWIN_OS_CASE_SENSITIVE_FIXME-2-false-alarm.patch", Size: 1784 bytes --]

From 7e3e51f4e288379be6a8731b739b63b9386940f7 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Thu, 18 May 2017 20:40:42 -0700
Subject: [PATCH 4/4] Fix DARWIN_OS_CASE_SENSITIVE_FIXME==2 false alarm
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* src/fileio.c (file_name_case_insensitive_p):
Don’t compile the (DARWIN_OS_CASE_SENSITIVE_FIXME == 2)
code unless DARWIN_OS_CASE_SENSITIVE_FIXME is 2.
Problem reported by Philipp Stephani in:
http://lists.gnu.org/archive/html/emacs-devel/2017-05/msg00495.html
---
 src/fileio.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/src/fileio.c b/src/fileio.c
index acbf76e..e5e3505 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -2294,10 +2294,14 @@ file_name_case_insensitive_p (const char *filename)
 		      & VOL_CAP_FMT_CASE_SENSITIVE);
 	}
     }
-  else if (DARWIN_OS_CASE_SENSITIVE_FIXME == 2)
+# if DARWIN_OS_CASE_SENSITIVE_FIXME == 2
     {
       /* The following is based on
-	 http://lists.apple.com/archives/darwin-dev/2007/Apr/msg00010.html.  */
+	 http://lists.apple.com/archives/darwin-dev/2007/Apr/msg00010.html.
+	 It is normally not even compiled, since it runs afoul of
+	 static checking.  See:
+	 http://lists.gnu.org/archive/html/emacs-devel/2017-05/msg00495.html
+         */
       struct attrlist alist;
       unsigned char buffer[sizeof (vol_capabilities_attr_t) + sizeof (size_t)];
 
@@ -2309,6 +2313,7 @@ file_name_case_insensitive_p (const char *filename)
       vol_capabilities_attr_t *vcaps = buffer;
       return !(vcaps->capabilities[0] & VOL_CAP_FMT_CASE_SENSITIVE);
     }
+# endif
 #endif	/* DARWIN_OS */
 
 #if defined CYGWIN || defined DOS_NT
-- 
2.7.4