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

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

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

On 05/19/2017 02:31 AM, Philipp Stephani wrote:
> Apparently on some systems mbstate_t is a nested struct, and the compiler 
> warns about missing braces. Note that memset to initialize a mbstate_t is 
> explicitly recommended in the libc manual:

Yes, of course memset works (which is all that the glibc manual really says). 
It's just that it's verbose and the verbosity isn't needed.

I guess the problem here is that clang's -Wmissing-braces option generates false 
alarms. The Clang folks may fix that someday. In the meantime I installed the 
first attached patch, to turn off that option for Clang.

> you could now add
> eassert (handlerlist == *dummy);
> or so to the cleanup function.

Thanks, good idea, installed in the 2nd attached patch.

> I think none of the four branches there work for macOS. 
> https://developer.apple.com/legacy/library/documentation/Darwin/Reference/ManPages/man2/pathconf.2.html is 
> silent about case sensitivity, so we're already relying on some undocumented 
> functionality. The getattrlist method is at least documented 
> (https://developer.apple.com/legacy/library/documentation/Darwin/Reference/ManPages/man2/getattrlist.2.html), 
> it just needs to be implemented correctly. I'd suggest to only use a working 
> version of getattrlist on macOS.

I installed that in the 3rd attached patch, at least I hope it works. But better 
yet, let's drop the getattrlist stuff entirely, as it doesn't seem to be needed 
and is just wasting our maintenance time. I installed that in the 4th attached 
patch.

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

From a5acb3701a7b4ab8b82aede308d28a47a383ae9c Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Fri, 19 May 2017 16:05:31 -0700
Subject: [PATCH 1/2] Port --enable-gcc-warnings to clang 3.9.1

* configure.ac (WERROR_CFLAGS): Omit -Wmissing-braces for Clang,
to shut off a false alarm.  Problem reportd by Philipp Stephani in:
http://lists.gnu.org/archive/html/emacs-devel/2017-05/msg00521.html
---
 configure.ac | 1 +
 1 file changed, 1 insertion(+)

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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Add-handlerlist-assertion-to-module-code.patch --]
[-- Type: text/x-patch; name="0002-Add-handlerlist-assertion-to-module-code.patch", Size: 1336 bytes --]

From 7d00410af69b3cbbf0e8fc9765f3bf9f5616286d Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Fri, 19 May 2017 16:15:07 -0700
Subject: [PATCH 2/2] Add handlerlist assertion to module code

* src/emacs-module.c (module_reset_handlerlist):
Check handlerlist.  Suggested by Philipp Stephani in:
http://lists.gnu.org/archive/html/emacs-devel/2017-05/msg00521.html
---
 src/emacs-module.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/emacs-module.c b/src/emacs-module.c
index 5aa8a88..0bc1b6c 100644
--- a/src/emacs-module.c
+++ b/src/emacs-module.c
@@ -918,10 +918,12 @@ finalize_environment (struct emacs_env_private *env)
 /* Must be called after setting up a handler immediately before
    returning from the function.  See the comments in lisp.h and the
    code in eval.c for details.  The macros below arrange for this
-   function to be called automatically.  DUMMY is ignored.  */
+   function to be called automatically.  PHANDLERLIST points to a word
+   containing the handler list, for sanity checking.  */
 static void
-module_reset_handlerlist (struct handler *const *dummy)
+module_reset_handlerlist (struct handler *const *phandlerlist)
 {
+  eassert (handlerlist == *phandlerlist);
   handlerlist = handlerlist->next;
 }
 
-- 
2.7.4


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0003-Narrow-DARWIN_OS_CASE_SENSITIVE_FIXME-to-1-choice.patch --]
[-- Type: text/x-patch; name="0003-Narrow-DARWIN_OS_CASE_SENSITIVE_FIXME-to-1-choice.patch", Size: 5240 bytes --]

From 075bd64609446e741a6efbcd6cd6e232db8d1df6 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sat, 20 May 2017 22:51:32 -0700
Subject: [PATCH 1/2] Narrow DARWIN_OS_CASE_SENSITIVE_FIXME to 1 choice

* etc/PROBLEMS: Document this (Bug#24441).
* src/fileio.c (file_name_case_insensitive_p): Prefer pathconf
with _PC_CASE_SENSITIVE, if it works, to
DARWIN_OS_CASE_SENSITIVE_FIXME code.
Support just one method for DARWIN_OS_CASE_SENSITIVE_FIXME,
which matches the Apple documentation more precisely.
---
 etc/PROBLEMS |  5 ++---
 src/fileio.c | 68 ++++++++++++++++++++----------------------------------------
 2 files changed, 24 insertions(+), 49 deletions(-)

diff --git a/etc/PROBLEMS b/etc/PROBLEMS
index e415887..ff88aa3 100644
--- a/etc/PROBLEMS
+++ b/etc/PROBLEMS
@@ -2486,9 +2486,8 @@ If you do, please send it to bug-gnu-emacs@gnu.org so we can list it here.
 The implementation of that function on Mac OS X uses pathconf with the
 _PC_CASE_SENSITIVE flag.  There have been reports that this use of
 pathconf does not work reliably.  If you have a problem, please
-recompile Emacs with -D DARWIN_OS_CASE_SENSITIVE_FIXME=1 or
--D DARWIN_OS_CASE_SENSITIVE_FIXME=2, and file a bug report saying
-whether this fixed your problem.
+recompile Emacs with -D DARWIN_OS_CASE_SENSITIVE_FIXME, and file a bug
+report saying whether this fixed your problem.
 
 * Build-time problems
 
diff --git a/src/fileio.c b/src/fileio.c
index e5e3505..17659b6 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -2256,65 +2256,41 @@ static bool
 file_name_case_insensitive_p (const char *filename)
 {
   /* Use pathconf with _PC_CASE_INSENSITIVE or _PC_CASE_SENSITIVE if
-     those flags are available.  As of this writing (2016-11-14),
+     those flags are available.  As of this writing (2017-05-20),
      Cygwin is the only platform known to support the former (starting
-     with Cygwin-2.6.1), and Mac OS X is the only platform known to
-     support the latter.
-
-     There have been reports that pathconf with _PC_CASE_SENSITIVE
-     does not work reliably on Mac OS X.  If you have a problem,
-     please recompile Emacs with -D DARWIN_OS_CASE_SENSITIVE_FIXME=1 or
-     -D DARWIN_OS_CASE_SENSITIVE_FIXME=2, and file a bug report saying
-     whether this fixed your problem.  */
+     with Cygwin-2.6.1), and macOS is the only platform known to
+     support the latter.  */
 
 #ifdef _PC_CASE_INSENSITIVE
   int res = pathconf (filename, _PC_CASE_INSENSITIVE);
   if (res >= 0)
     return res > 0;
-#elif defined _PC_CASE_SENSITIVE && !defined DARWIN_OS_CASE_SENSITIVE_FIXME
+#elif defined _PC_CASE_SENSITIVE
   int res = pathconf (filename, _PC_CASE_SENSITIVE);
   if (res >= 0)
     return res == 0;
 #endif
 
-#ifdef DARWIN_OS
-# ifndef DARWIN_OS_CASE_SENSITIVE_FIXME
-  int DARWIN_OS_CASE_SENSITIVE_FIXME = 0;
-# endif
+  /* There have been reports that pathconf with _PC_CASE_SENSITIVE
+     does not work reliably on Mac OS X.  If you have a problem,
+     please recompile Emacs with -D DARWIN_OS_CASE_SENSITIVE_FIXME=1 or
+     -D DARWIN_OS_CASE_SENSITIVE_FIXME=2, and file a bug report saying
+     whether this fixed your problem.  */
 
-  if (DARWIN_OS_CASE_SENSITIVE_FIXME == 1)
-    {
-      /* This is based on developer.apple.com's getattrlist man page.  */
-      struct attrlist alist = {.volattr = ATTR_VOL_CAPABILITIES};
-      vol_capabilities_attr_t vcaps;
-      if (getattrlist (filename, &alist, &vcaps, sizeof vcaps, 0) == 0)
-	{
-	  if (vcaps.valid[VOL_CAPABILITIES_FORMAT] & VOL_CAP_FMT_CASE_SENSITIVE)
-	    return ! (vcaps.capabilities[VOL_CAPABILITIES_FORMAT]
-		      & VOL_CAP_FMT_CASE_SENSITIVE);
-	}
-    }
-# if DARWIN_OS_CASE_SENSITIVE_FIXME == 2
-    {
-      /* The following is based on
-	 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)];
-
-      memset (&alist, 0, sizeof (alist));
-      alist.volattr = ATTR_VOL_CAPABILITIES;
-      if (getattrlist (filename, &alist, buffer, sizeof (buffer), 0)
-	  || !(alist.volattr & ATTR_VOL_CAPABILITIES))
-	return 0;
-      vol_capabilities_attr_t *vcaps = buffer;
-      return !(vcaps->capabilities[0] & VOL_CAP_FMT_CASE_SENSITIVE);
-    }
+#ifdef DARWIN_OS_CASE_SENSITIVE_FIXME
+# ifdef VOL_CAP_FMT_CASE_SENSITIVE
+  {
+    struct attrlist alist = {.bitmapcount = ATTR_BIT_MAP_COUNT,
+			     .volattr = ATTR_VOL_INFO | ATTR_VOL_CAPABILITIES};
+    struct { uint32_t len; vol_capabilities_attr_t caps; } vcaps
+      __attribute__ ((aligned (4), packed));
+    int i = VOL_CAPABILITIES_FORMAT;
+    if (getattrlist (filename, &alist, &vcaps, sizeof vcaps, 0) == 0
+	&& (vcaps.caps.valid[i] & VOL_CAP_FMT_CASE_SENSITIVE))
+      return ! (vcaps.caps.capabilities[i] & VOL_CAP_FMT_CASE_SENSITIVE);
+  }
 # endif
-#endif	/* DARWIN_OS */
+#endif
 
 #if defined CYGWIN || defined DOS_NT
   return true;
-- 
2.7.4


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #5: 0004-Remove-DARWIN_OS_CASE_SENSITIVE_FIXME-code.patch --]
[-- Type: text/x-patch; name="0004-Remove-DARWIN_OS_CASE_SENSITIVE_FIXME-code.patch", Size: 2542 bytes --]

From b35293dfd0e9dd95a88ac01051655d0d2d105992 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sat, 20 May 2017 22:55:17 -0700
Subject: [PATCH 2/2] Remove DARWIN_OS_CASE_SENSITIVE_FIXME code

It does not appear to be needed (Bug#24441).
* etc/PROBLEMS: Remove DARWIN_OS_CASE_SENSITIVE_FIXME stuff.
* src/fileio.c (file_name_case_insensitive_p):
Remove DARWIN_OS_CASE_SENSITIVE_FIXME code.
---
 etc/PROBLEMS | 10 ----------
 src/fileio.c | 21 ---------------------
 2 files changed, 31 deletions(-)

diff --git a/etc/PROBLEMS b/etc/PROBLEMS
index ff88aa3..593eb6b 100644
--- a/etc/PROBLEMS
+++ b/etc/PROBLEMS
@@ -2479,16 +2479,6 @@ please call support for your X-server and see if you can get a fix.
 If you do, please send it to bug-gnu-emacs@gnu.org so we can list it here.
 
 
-* Runtime problems specific to Mac OS X
-
-** On Mac OS X, file-name-case-insensitive-p may be unreliable
-
-The implementation of that function on Mac OS X uses pathconf with the
-_PC_CASE_SENSITIVE flag.  There have been reports that this use of
-pathconf does not work reliably.  If you have a problem, please
-recompile Emacs with -D DARWIN_OS_CASE_SENSITIVE_FIXME, and file a bug
-report saying whether this fixed your problem.
-
 * Build-time problems
 
 ** Configuration
diff --git a/src/fileio.c b/src/fileio.c
index 17659b6..c21056e 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -2271,27 +2271,6 @@ file_name_case_insensitive_p (const char *filename)
     return res == 0;
 #endif
 
-  /* There have been reports that pathconf with _PC_CASE_SENSITIVE
-     does not work reliably on Mac OS X.  If you have a problem,
-     please recompile Emacs with -D DARWIN_OS_CASE_SENSITIVE_FIXME=1 or
-     -D DARWIN_OS_CASE_SENSITIVE_FIXME=2, and file a bug report saying
-     whether this fixed your problem.  */
-
-#ifdef DARWIN_OS_CASE_SENSITIVE_FIXME
-# ifdef VOL_CAP_FMT_CASE_SENSITIVE
-  {
-    struct attrlist alist = {.bitmapcount = ATTR_BIT_MAP_COUNT,
-			     .volattr = ATTR_VOL_INFO | ATTR_VOL_CAPABILITIES};
-    struct { uint32_t len; vol_capabilities_attr_t caps; } vcaps
-      __attribute__ ((aligned (4), packed));
-    int i = VOL_CAPABILITIES_FORMAT;
-    if (getattrlist (filename, &alist, &vcaps, sizeof vcaps, 0) == 0
-	&& (vcaps.caps.valid[i] & VOL_CAP_FMT_CASE_SENSITIVE))
-      return ! (vcaps.caps.capabilities[i] & VOL_CAP_FMT_CASE_SENSITIVE);
-  }
-# endif
-#endif
-
 #if defined CYGWIN || defined DOS_NT
   return true;
 #else
-- 
2.7.4