From: Paul Eggert <eggert@cs.ucla.edu>
To: Philipp Stephani <phst@google.com>,
Philipp Stephani <p.stephani2@gmail.com>,
emacs-devel@gnu.org
Subject: Re: [PATCH] Clean up a couple of compiler warnings
Date: Sat, 20 May 2017 23:07:34 -0700 [thread overview]
Message-ID: <c3033bed-ef38-98f9-7eff-10a0ebfddb49@cs.ucla.edu> (raw)
In-Reply-To: <CAP-RRPtjkhwHjvMLx4AD=HhMJgWwMOZcUoPe87+zsoe3v7nkng@mail.gmail.com>
[-- 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
next prev parent reply other threads:[~2017-05-21 6:07 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-05-18 20:24 [PATCH] Clean up a couple of compiler warnings Philipp Stephani
2017-05-18 20:40 ` Eli Zaretskii
2017-05-19 3:48 ` Paul Eggert
2017-05-19 6:53 ` Eli Zaretskii
2017-05-19 6:55 ` Paul Eggert
2017-05-19 9:31 ` Philipp Stephani
2017-05-21 6:07 ` Paul Eggert [this message]
2017-05-21 20:00 ` Philipp Stephani
2017-05-21 20:33 ` Paul Eggert
2017-05-23 10:17 ` Philipp Stephani
2017-05-23 17:36 ` Paul Eggert
2017-05-23 19:19 ` Eli Zaretskii
2017-05-23 19:21 ` Philipp Stephani
2017-05-21 20:47 ` Perry E. Metzger
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=c3033bed-ef38-98f9-7eff-10a0ebfddb49@cs.ucla.edu \
--to=eggert@cs.ucla.edu \
--cc=emacs-devel@gnu.org \
--cc=p.stephani2@gmail.com \
--cc=phst@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.