Re: xalloc: missing prototype

[Akim Demaille]

Hi Paul,

Le 6 déc. 2012 à 18:48, Paul Eggert  a écrit :

> In file included from ../../../lib/mbschr.c:23:0:
> ../../../lib/mbschr.c: At top level:
> ../../../lib/mbuiter.h:201:181: warning: '__inline_memset_chk' is static but 
> used in inline function 'mbuiter_multi_copy' which is not static [enabled by 
> default]
> memset (&new_iter->state, 0, sizeof (mbstate_t));
> 
> This (and similar warnings) appear to be a bug in the
> build environment.  It ought to be OK
> to invoke memset from an extern inline function.
> My guess is that the warning can be safely ignored.
> You might want to file a bug report with Apple or
> whoever.

That's good old GCC, every version I can run from 4.3 to 4.8.
This is a stripped down version of the code from gnulib on
my platform.

$ cat mbschr.i
static __inline int
iswcntrl(int wc)
{
  return wc;
}

inline int
mb_width_aux (int wc)
{
  return iswcntrl (wc);
}
$ gcc-mp-4.3  -c mbschr.i
$ gcc-mp-4.3 -std=gnu99 -c mbschr.i
mbschr.i: In function 'mb_width_aux':
mbschr.i:10: warning: 'iswcntrl' is static but used in inline function 
'mb_width_aux' which is not static
$ gcc-mp-4.3 -std=c99 -c mbschr.i
mbschr.i: In function 'mb_width_aux':
mbschr.i:10: warning: 'iswcntrl' is static but used in inline function 
'mb_width_aux' which is not static
$ gcc-mp-4.8 -std=gnu99 -c mbschr.i
mbschr.i:10:10: warning: 'iswcntrl' is static but used in inline function 
'mb_width_aux' which is not static [enabled by default]
   return iswcntrl (wc);
  ^

The prototype comes from /usr/include/_wctype.h:


> #ifndef __DARWIN_WCTYPE_TOP_static_inline
> #define __DARWIN_WCTYPE_TOP_static_inline static __inline
> #endif
> 
...
> 
> __DARWIN_WCTYPE_TOP_static_inline int
> iswcntrl(wint_t _wc)
> {
>   return (__istype(_wc, _CTYPE_C));
> }

The warning is legitimate, as 6.7.4.3 in C99 reads:

• An inline definition of a function with external linkage shall not 
contain a definition of a modifiable object with static storage duration, and 
shall not contain a reference to an identifier with internal linkage.

The problem is the prototype of iswcntrl which is incompatible
with the use in an inline function.  Yet I don't understand
why GCC does not accept that inline functions call static
functions that are inline too.

Re: xalloc: missing prototype

[Akim Demaille]

Le 7 déc. 2012 à 01:18, Jim Meyering  a écrit :

> Hi Akim,
> 
> I've turned off -Wcast-qual warning for coreutils, grep, diffutils, etc.
> For us, that seems to be the best route, since we try hard not to
> add new casts (so there's little risk of introducing new violations),
> and since the few code sites that provoke warnings have already been
> carefully examined.

Hi Jim!

OK, thanks.  Let it be for Bison (maint) too:

commit 28d16d1f7cd7ae201d368210ba87f3ef5c0abd51
Author: Akim Demaille 
Date:   Fri Dec 7 09:48:41 2012 +0100

build: drop -Wcast-qual

Suggested by Jim Meyering.
http://lists.gnu.org/archive/html/bug-gnulib/2012-12/msg00017.html
* configure.ac (warn_common): Remove -Wcast-qual.

diff --git a/configure.ac b/configure.ac
index 83e20ac..4004f55 100644
--- a/configure.ac
+++ b/configure.ac
@@ -67,7 +67,7 @@ AC_ARG_ENABLE([gcc-warnings],
   [enable_gcc_warnings=no])
 if test "$enable_gcc_warnings" = yes; then
   warn_common='-Wall -Wextra -Wno-sign-compare -Wcast-align
--Wcast-qual -Wformat -Wpointer-arith -Wwrite-strings'
+-Wformat -Wpointer-arith -Wwrite-strings'
   warn_c='-Wbad-function-cast -Wmissing-declarations -Wmissing-prototypes
 -Wshadow -Wstrict-prototypes'
   warn_cxx='-Wnoexcept'

Re: xalloc: missing prototype

[Akim Demaille]

Le 5 déc. 2012 à 17:19, Eric Blake  a écrit :

> What I have instead done in libvirt to allow compilation with older gcc
> is the following:
> 
># Gnulib uses '#pragma GCC diagnostic push' to silence some
># warnings, but older gcc doesn't support this.
>AC_CACHE_CHECK([whether pragma GCC diagnostic push works],
>  [lv_cv_gcc_pragma_push_works], [
>  save_CFLAGS=$CFLAGS
>  CFLAGS='-Wunknown-pragmas -Werror'
>  AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
>#pragma GCC diagnostic push
>#pragma GCC diagnostic pop
>  ]])],
>  [lv_cv_gcc_pragma_push_works=yes],
>  [lv_cv_gcc_pragma_push_works=no])
>  CFLAGS=$save_CFLAGS])
>if test $lv_cv_gcc_pragma_push_works = no; then
>  dontwarn="$dontwarn -Wmissing-prototypes"
>  dontwarn="$dontwarn -Wmissing-declarations"
>fi

Thanks Eric, I installed this in Bison (maint):

commit 85a2f27fc4e271e992ca35561ae93c55aac21c07
Author: Akim Demaille 
Date:   Fri Dec 7 09:58:40 2012 +0100

build: keep -Wmissing-declarations and -Wmissing-prototypes for modern GCCs

Fixes a -Werror failure of xalloc.h used in src.
From Eric Blake.
http://lists.gnu.org/archive/html/bug-gnulib/2012-12/msg6.html

* configure.ac: Check whether GCC pragma diagnostic push/pop works.
Enable these warnings for bison if it does.
Enable these warnings for the test suite anyway.

diff --git a/configure.ac b/configure.ac
index 4004f55..d14cb29 100644
--- a/configure.ac
+++ b/configure.ac
@@ -58,6 +58,20 @@ AC_PROG_CXX
 # Gnulib (early checks).
 gl_EARLY
 
+# Gnulib uses '#pragma GCC diagnostic push' to silence some
+# warnings, but older gcc doesn't support this.
+AC_CACHE_CHECK([whether pragma GCC diagnostic push works],
+  [lv_cv_gcc_pragma_push_works], [
+  save_CFLAGS=$CFLAGS
+  CFLAGS='-Wunknown-pragmas -Werror'
+  AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
+#pragma GCC diagnostic push
+#pragma GCC diagnostic pop
+  ]])],
+  [lv_cv_gcc_pragma_push_works=yes],
+  [lv_cv_gcc_pragma_push_works=no])
+  CFLAGS=$save_CFLAGS])
+
 AC_ARG_ENABLE([gcc-warnings],
 [  --enable-gcc-warnings   turn on lots of GCC warnings (not recommended)],
 [case $enable_gcc_warnings in
@@ -68,8 +82,7 @@ AC_ARG_ENABLE([gcc-warnings],
 if test "$enable_gcc_warnings" = yes; then
   warn_common='-Wall -Wextra -Wno-sign-compare -Wcast-align
 -Wformat -Wpointer-arith -Wwrite-strings'
-  warn_c='-Wbad-function-cast -Wmissing-declarations -Wmissing-prototypes
--Wshadow -Wstrict-prototypes'
+  warn_c='-Wbad-function-cast -Wshadow -Wstrict-prototypes'
   warn_cxx='-Wnoexcept'
   AC_LANG_PUSH([C])
   # Clang supports many of GCC's -W options, but only issues warnings
@@ -89,12 +102,21 @@ if test "$enable_gcc_warnings" = yes; then
 gl_WARN_ADD([$i], [WARN_CFLAGS])
   done
   gl_WARN_ADD([-Werror], [WERROR_CFLAGS])
+
+  # Warnings for the test suite, and maybe for bison if GCC is modern
+  # enough.
+  gl_WARN_ADD([-Wmissing-declarations], [WARN_CFLAGS_TEST])
+  gl_WARN_ADD([-Wmissing-prototypes], [WARN_CFLAGS_TEST])
+  test $lv_cv_gcc_pragma_push_works = yes &&
+AS_VAR_APPEND([WARN_CFLAGS], [" $WARN_CFLAGS_TEST"])
+
   # Warnings for the test suite only.
   gl_WARN_ADD([-Wundef], [WARN_CFLAGS_TEST])
   gl_WARN_ADD([-pedantic], [WARN_CFLAGS_TEST])
   CFLAGS=$save_CFLAGS
   AC_LANG_POP([C])
 
+
   AC_LANG_PUSH([C++])
   save_CXXFLAGS=$CXXFLAGS
   gl_WARN_ADD([-Werror=unknown-warning-option], [CXXFLAGS])

Re: [sharutils-4.11.1] Compilation warnings

[Paul Eggert]

On 12/06/2012 08:53 PM, Bruce Korb wrote:
> can GCC decide that the result of (x) is not really used and still complain?

GCC is allowed to generate whatever warnings it likes.
It can warn whenever you have a program that uses
the letter 'a', say.  But it is required to generate
code that does what you want, for that case.

Re: xalloc: missing prototype

[Paul Eggert]

On 12/07/2012 12:45 AM, Akim Demaille wrote:

> I don't understand
> why GCC does not accept that inline functions call static
> functions that are inline too.

The C Standard requires a diagnostic here, as the rule that you
quoted is a constraint.  So this is really more a question for
the developers of the C Standard, not for the GCC maintainers.

Anyhow, clearly this is a bug in Darwin.  Standard C headers cannot
use 'static inline' to define functions that might be called by
user code, as the user code might be in an extern inline function,
which can't call static inline functions.

If you're a Darwin user can you please report this bug?
Also, does the following patch work around the problem for you?
If so, I'll push it into gnulib.

Also, Gnulib itself suffers from this problem in some of its .h
files -- I'll try to take a look at that when I find the time.

>From 9fb0699950f4ae2541612a6f5d9c47197b6b25c3 Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Fri, 7 Dec 2012 09:31:21 -0800
Subject: [PATCH] extern-inline: avoid incompatibility with Darwin Libc

* m4/extern-inline.m4 (_GL_INLINE, _GL_EXTERN_INLINE) [__APPLE__]:
Do not use C99 extern inline.
---
 ChangeLog   |  6 ++
 m4/extern-inline.m4 | 14 ++
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index fb6f84e..5f67f15 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2012-12-07  Paul Eggert  
+
+   extern-inline: avoid incompatibility with Darwin Libc
+   * m4/extern-inline.m4 (_GL_INLINE, _GL_EXTERN_INLINE) [__APPLE__]:
+   Do not use C99 extern inline.
+
 2012-12-05  Paul Eggert  
 
list, oset, xlist, xoset: fix extern inline issue with C99
diff --git a/m4/extern-inline.m4 b/m4/extern-inline.m4
index 2492260..6cefba3 100644
--- a/m4/extern-inline.m4
+++ b/m4/extern-inline.m4
@@ -16,10 +16,16 @@ AC_DEFUN([gl_EXTERN_INLINE],
  when FOO is an inline function in the header; see
  .
_GL_INLINE_HEADER_END contains useful stuff to put
- in the same include file, after uses of _GL_INLINE.  */
-#if (__GNUC__ \
- ? defined __GNUC_STDC_INLINE__ && __GNUC_STDC_INLINE__ \
- : 199901L <= __STDC_VERSION__)
+ in the same include file, after uses of _GL_INLINE.
+
+   Suppress the use of C99 extern inline on Apple's platforms,
+   as Libc-825.25 (2012-09-19) is incompatible with it; see
+   .
+   Perhaps Apple will fix this some day.  */
+#if ((__GNUC__ \
+  ? defined __GNUC_STDC_INLINE__ && __GNUC_STDC_INLINE__ \
+  : 199901L <= __STDC_VERSION__) \
+ && !defined __APPLE__)
 # define _GL_INLINE inline
 # define _GL_EXTERN_INLINE extern inline
 #elif 2 < __GNUC__ + (7 <= __GNUC_MINOR__)
-- 
1.7.11.7

Re: ASCII_ONLY?

[Paul Eggert]

On 12/06/12 21:44, Ben Pfaff wrote:
> But I don't see any actual uses of this macro.  What is the
> intent?

My guess is was to distinguish between hosts where the
C locale supports only ASCII characters, and hosts where
the C locale is UTF-8.  If the macro isn't being used now,
it should be safe to remove its definitions.

[PATCH v2] mountlist: additional dummy FS names for Linux

[Eric Wong]

* lib/mountlist.c (ME_DUMMY_0):
  additional dummy FS names for Linux systems.
  - "devpts" PTY slave filesystem
  - "fusectl" control filesystem for FUSE
  - "mqueue" enumerates POSIX message queues
  - "rpc_pipefs" kernel <-> userspace bridge for NFS
  - "sysfs" is for exporting kernel objects
  - "devfs" device filesystem for Linux 2.4 and FreeBSD
  There are likely more dummy FSes I am not aware of.
---
 Since my original patch, I've added "devfs"

 lib/mountlist.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/lib/mountlist.c b/lib/mountlist.c
index d0fe1b2..359aa10 100644
--- a/lib/mountlist.c
+++ b/lib/mountlist.c
@@ -153,6 +153,14 @@
   (strcmp (Fs_type, "autofs") == 0  \
|| strcmp (Fs_type, "proc") == 0 \
|| strcmp (Fs_type, "subfs") == 0\
+   /* for Linux 2.6/3.x */  \
+   || strcmp (Fs_type, "devpts") == 0   \
+   || strcmp (Fs_type, "fusectl") == 0  \
+   || strcmp (Fs_type, "mqueue") == 0   \
+   || strcmp (Fs_type, "rpc_pipefs") == 0   \
+   || strcmp (Fs_type, "sysfs") == 0\
+   /* FreeBSD, Linux 2.4 */ \
+   || strcmp (Fs_type, "devfs") == 0\
/* for NetBSD 3.0 */ \
|| strcmp (Fs_type, "kernfs") == 0   \
/* for Irix 6.5 */   \
-- 
Eric Wong

Re: [PATCH v2] mountlist: additional dummy FS names for Linux

[Jim Meyering]

Eric Wong wrote:
> * lib/mountlist.c (ME_DUMMY_0):
>   additional dummy FS names for Linux systems.
>   - "devpts" PTY slave filesystem
>   - "fusectl" control filesystem for FUSE
>   - "mqueue" enumerates POSIX message queues
>   - "rpc_pipefs" kernel <-> userspace bridge for NFS
>   - "sysfs" is for exporting kernel objects
>   - "devfs" device filesystem for Linux 2.4 and FreeBSD
>   There are likely more dummy FSes I am not aware of.
> ---
>  Since my original patch, I've added "devfs"
>
>  lib/mountlist.c | 8 
>  1 file changed, 8 insertions(+)
>
> diff --git a/lib/mountlist.c b/lib/mountlist.c
> index d0fe1b2..359aa10 100644
> --- a/lib/mountlist.c
> +++ b/lib/mountlist.c
> @@ -153,6 +153,14 @@
>(strcmp (Fs_type, "autofs") == 0  \
> || strcmp (Fs_type, "proc") == 0 \
> || strcmp (Fs_type, "subfs") == 0\
> +   /* for Linux 2.6/3.x */  \
> +   || strcmp (Fs_type, "devpts") == 0   \
> +   || strcmp (Fs_type, "fusectl") == 0  \
> +   || strcmp (Fs_type, "mqueue") == 0   \
> +   || strcmp (Fs_type, "rpc_pipefs") == 0   \
> +   || strcmp (Fs_type, "sysfs") == 0\
> +   /* FreeBSD, Linux 2.4 */ \
> +   || strcmp (Fs_type, "devfs") == 0\
> /* for NetBSD 3.0 */ \
> || strcmp (Fs_type, "kernfs") == 0   \
> /* for Irix 6.5 */   \

Thanks.  I'll look at this Sat or Sunday.
What's the motivation?  Have df hide those entries?

Re: [PATCH v2] mountlist: additional dummy FS names for Linux

[Eric Wong]

Jim Meyering  wrote:
> Eric Wong wrote:
> > * lib/mountlist.c (ME_DUMMY_0):
> >   additional dummy FS names for Linux systems.
> >   - "devpts" PTY slave filesystem
> >   - "fusectl" control filesystem for FUSE
> >   - "mqueue" enumerates POSIX message queues
> >   - "rpc_pipefs" kernel <-> userspace bridge for NFS
> >   - "sysfs" is for exporting kernel objects
> >   - "devfs" device filesystem for Linux 2.4 and FreeBSD
> >   There are likely more dummy FSes I am not aware of.
> 
> Thanks.  I'll look at this Sat or Sunday.
> What's the motivation?  Have df hide those entries?

It just seems right to mark those as dummy since they
aren't used for normal file storage.

It allows cmogstored[1] to skip some entries and avoid extra
stat() and a tiny amount of memory allocation.  No big deal
with or without it.

[1] http://bogomips.org/cmogstored/README

-- 
Eric Wong

Re: ASCII_ONLY?

[Bruno Haible]

Hi Ben,

> gnulib has three definitions of ASCII_ONLY in files that #include
> "vasnprintf.c":
> 
> lib/unistdio/u16-vasnprintf.c:#define ASCII_ONLY 1
> lib/unistdio/u32-vasnprintf.c:#define ASCII_ONLY 1
> lib/unistdio/u8-vasnprintf.c:#define ASCII_ONLY 1
> 
> But I don't see any actual uses of this macro.  What is the
> intent?  (Is it related to FCHAR_T_ONLY_ASCII?)

Bingo! The comments in vasnprintf.c say:

 FCHAR_T_ONLY_ASCII Set to 1 to enable verification that all characters
in the format string are ASCII. MUST be set if
FCHAR_T and DCHAR_T are not the same type.

And in lib/unistdio/u{8,16,32}-vasnprintf.c FCHAR_T and DCHAR_T are indeed
not the same type. Therefore the 3 files should define FCHAR_T_ONLY_ASCII,
not ASCII_ONLY.

I would be grateful to you if you could commit the obvious fix.

Did you find this by code inspection, or through a gcc or clang warning?

Bruno

Re: ASCII_ONLY?

[Paul Eggert]

On 12/07/2012 06:40 PM, Bruno Haible wrote:
> I would be grateful to you if you could commit the obvious fix.

Done, thanks.

Re: ASCII_ONLY?

[Ben Pfaff]

Bruno Haible  writes:

>> gnulib has three definitions of ASCII_ONLY in files that #include
>> "vasnprintf.c":
>> 
>> lib/unistdio/u16-vasnprintf.c:#define ASCII_ONLY 1
>> lib/unistdio/u32-vasnprintf.c:#define ASCII_ONLY 1
>> lib/unistdio/u8-vasnprintf.c:#define ASCII_ONLY 1
>> 
>> But I don't see any actual uses of this macro.  What is the
>> intent?  (Is it related to FCHAR_T_ONLY_ASCII?)
>
> Bingo! The comments in vasnprintf.c say:
>
>  FCHAR_T_ONLY_ASCII Set to 1 to enable verification that all characters
> in the format string are ASCII. MUST be set if
> FCHAR_T and DCHAR_T are not the same type.
>
> And in lib/unistdio/u{8,16,32}-vasnprintf.c FCHAR_T and DCHAR_T are indeed
> not the same type. Therefore the 3 files should define FCHAR_T_ONLY_ASCII,
> not ASCII_ONLY.
>
> I would be grateful to you if you could commit the obvious fix.
>
> Did you find this by code inspection, or through a gcc or clang warning?

I found it by code inspection.  Working on a new user of vasnprintf.c
(see http://lists.gnu.org/archive/html/bug-gnulib/2012-12/msg00022.html),
and examining the existing users, I saw the definition of ASCII_ONLY,
but couldn't figure out what it was good for.

Thanks, Bruno and Paul.