[libvirt] [PATCH v2] virsh: workaround readline prototypes warnings

When building with clang 4.0.0, virsh build fails like this: gmake[3]: Entering directory '/usr/home/novel/code/libvirt/tools' CC virsh-virsh.o In file included from virsh.c:45: In file included from /usr/local/include/readline/readline.h:31: /usr/local/include/readline/rltypedefs.h:35:22: error: this function declaration is not a prototype [-Werror,-Wstrict-prototypes] typedef int Function () __attribute__ ((deprecated)); ^ void /usr/local/include/readline/rltypedefs.h:36:24: error: this function declaration is not a prototype [-Werror,-Wstrict-prototypes] typedef void VFunction () __attribute__ ((deprecated)); ^ void /usr/local/include/readline/rltypedefs.h:37:26: error: this function declaration is not a prototype [-Werror,-Wstrict-prototypes] typedef char *CPFunction () __attribute__ ((deprecated)); ^ void /usr/local/include/readline/rltypedefs.h:38:28: error: this function declaration is not a prototype [-Werror,-Wstrict-prototypes] typedef char **CPPFunction () __attribute__ ((deprecated)); ^ void In file included from virsh.c:45: /usr/local/include/readline/readline.h:385:23: error: this function declaration is not a prototype [-Werror,-Wstrict-prototypes] extern int rl_message (); ^ void 5 errors generated. gmake[3]: *** [Makefile:2823: virsh-virsh.o] Error 1 Fix that by adding -D_FUNCTION_DEF to READLINE_CFLAGS to fix *Function related warnings and add a check for stdarg.h so readline headers use proper rl_message declaration. Bug report on the readline mailing list: http://lists.gnu.org/archive/html/bug-readline/2017-05/msg00004.html --- configure.ac | 2 +- m4/virt-readline.m4 | 4 +++- tools/Makefile.am | 3 ++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/configure.ac b/configure.ac index 1af5538ee..1a73b3466 100644 --- a/configure.ac +++ b/configure.ac @@ -328,7 +328,7 @@ dnl Availability of various common headers (non-fatal if missing). AC_CHECK_HEADERS([pwd.h regex.h sys/un.h \ sys/poll.h syslog.h mntent.h net/ethernet.h linux/magic.h \ sys/un.h sys/syscall.h sys/sysctl.h netinet/tcp.h ifaddrs.h \ - libtasn1.h sys/ucred.h sys/mount.h]) + libtasn1.h sys/ucred.h sys/mount.h stdarg.h]) dnl Check whether endian provides handy macros. AC_CHECK_DECLS([htole64], [], [], [[#include <endian.h>]]) AC_CHECK_FUNCS([stat stat64 __xstat __xstat64 lstat lstat64 __lxstat __lxstat64]) diff --git a/m4/virt-readline.m4 b/m4/virt-readline.m4 index ee70ea36b..9fba5148a 100644 --- a/m4/virt-readline.m4 +++ b/m4/virt-readline.m4 @@ -45,7 +45,9 @@ AC_DEFUN([LIBVIRT_CHECK_READLINE],[ # function, to ensure we aren't being confused by caching. LIBS=$lv_saved_libs AC_CHECK_LIB([readline], [rl_initialize], - [], [READLINE_LIBS="$READLINE_LIBS $extra_LIBS"]) + [READLINE_CFLAGS="-D_FUNCTION_DEF $READLINE_CFLAGS" + AC_SUBST(READLINE_CFLAGS)], + [READLINE_LIBS="$READLINE_LIBS $extra_LIBS"]) LIBS=$lv_saved_libs ]) diff --git a/tools/Makefile.am b/tools/Makefile.am index 56691c289..345521457 100644 --- a/tools/Makefile.am +++ b/tools/Makefile.am @@ -244,7 +244,8 @@ virsh_CFLAGS = \ $(WARN_CFLAGS) \ $(PIE_CFLAGS) \ $(COVERAGE_CFLAGS) \ - $(LIBXML_CFLAGS) + $(LIBXML_CFLAGS) \ + $(READLINE_CFLAGS) virt_admin_SOURCES = \ virt-admin.c virt-admin.h \ -- 2.13.0

On Fri, Jun 02, 2017 at 07:04:23PM +0400, Roman Bogorodskiy wrote:
When building with clang 4.0.0, virsh build fails like this:
[...]
Bug report on the readline mailing list:
http://lists.gnu.org/archive/html/bug-readline/2017-05/msg00004.html --- configure.ac | 2 +- m4/virt-readline.m4 | 4 +++- tools/Makefile.am | 3 ++- 3 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/configure.ac b/configure.ac index 1af5538ee..1a73b3466 100644 --- a/configure.ac +++ b/configure.ac @@ -328,7 +328,7 @@ dnl Availability of various common headers (non-fatal if missing). AC_CHECK_HEADERS([pwd.h regex.h sys/un.h \ sys/poll.h syslog.h mntent.h net/ethernet.h linux/magic.h \ sys/un.h sys/syscall.h sys/sysctl.h netinet/tcp.h ifaddrs.h \ - libtasn1.h sys/ucred.h sys/mount.h]) + libtasn1.h sys/ucred.h sys/mount.h stdarg.h]) dnl Check whether endian provides handy macros. AC_CHECK_DECLS([htole64], [], [], [[#include <endian.h>]]) AC_CHECK_FUNCS([stat stat64 __xstat __xstat64 lstat lstat64 __lxstat __lxstat64])
What is the purpose of checking for stdarg.h? ACK with this hunk removed or explained. Jan

Ján Tomko wrote:
On Fri, Jun 02, 2017 at 07:04:23PM +0400, Roman Bogorodskiy wrote:
When building with clang 4.0.0, virsh build fails like this:
[...]
Bug report on the readline mailing list:
http://lists.gnu.org/archive/html/bug-readline/2017-05/msg00004.html --- configure.ac | 2 +- m4/virt-readline.m4 | 4 +++- tools/Makefile.am | 3 ++- 3 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/configure.ac b/configure.ac index 1af5538ee..1a73b3466 100644 --- a/configure.ac +++ b/configure.ac @@ -328,7 +328,7 @@ dnl Availability of various common headers (non-fatal if missing). AC_CHECK_HEADERS([pwd.h regex.h sys/un.h \ sys/poll.h syslog.h mntent.h net/ethernet.h linux/magic.h \ sys/un.h sys/syscall.h sys/sysctl.h netinet/tcp.h ifaddrs.h \ - libtasn1.h sys/ucred.h sys/mount.h]) + libtasn1.h sys/ucred.h sys/mount.h stdarg.h]) dnl Check whether endian provides handy macros. AC_CHECK_DECLS([htole64], [], [], [[#include <endian.h>]]) AC_CHECK_FUNCS([stat stat64 __xstat __xstat64 lstat lstat64 __lxstat __lxstat64])
What is the purpose of checking for stdarg.h?
As the commit message says: "... and add a check for stdarg.h so readline headers use proper rl_message declaration." I thought that's enough details, but obviously it's not as you're asking :-) The thing is that there's a following code in readline.h: #if defined (USE_VARARGS) && defined (PREFER_STDARG) extern int rl_message (const char *, ...) __attribute__((__format__ (printf, 1, 2))); #else extern int rl_message (); #endif The latter generates the described warning and the former does not. PREFER_STDARG and USE_VARARGS come from another readline header, rlstdc.h, and it contains the following: #if defined (__STDC__) && defined (HAVE_STDARG_H) # define PREFER_STDARG # define USE_VARARGS #else # if defined (HAVE_VARARGS_H) # define PREFER_VARARGS # define USE_VARARGS # endif #endif So we need to have HAVE_STDARG_H defined to have a proper declaration. How do you like if I rephrase it like "... and add a check for stdarg.h so we have HAVE_STDARG_H defined that's needed by the readline headers to use proper rl_message declaration"?
ACK with this hunk removed or explained.
Jan
Roman Bogorodskiy

On Thu, Jun 08, 2017 at 05:20:35PM +0400, Roman Bogorodskiy wrote:
Ján Tomko wrote:
On Fri, Jun 02, 2017 at 07:04:23PM +0400, Roman Bogorodskiy wrote:
When building with clang 4.0.0, virsh build fails like this:
[...]
Bug report on the readline mailing list:
http://lists.gnu.org/archive/html/bug-readline/2017-05/msg00004.html --- configure.ac | 2 +- m4/virt-readline.m4 | 4 +++- tools/Makefile.am | 3 ++- 3 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/configure.ac b/configure.ac index 1af5538ee..1a73b3466 100644 --- a/configure.ac +++ b/configure.ac @@ -328,7 +328,7 @@ dnl Availability of various common headers (non-fatal if missing). AC_CHECK_HEADERS([pwd.h regex.h sys/un.h \ sys/poll.h syslog.h mntent.h net/ethernet.h linux/magic.h \ sys/un.h sys/syscall.h sys/sysctl.h netinet/tcp.h ifaddrs.h \ - libtasn1.h sys/ucred.h sys/mount.h]) + libtasn1.h sys/ucred.h sys/mount.h stdarg.h]) dnl Check whether endian provides handy macros. AC_CHECK_DECLS([htole64], [], [], [[#include <endian.h>]]) AC_CHECK_FUNCS([stat stat64 __xstat __xstat64 lstat lstat64 __lxstat __lxstat64])
What is the purpose of checking for stdarg.h?
As the commit message says: "... and add a check for stdarg.h so readline headers use proper rl_message declaration." I thought that's enough details, but obviously it's not as you're asking :-)
I skipped that part of the commit message. I did not expect that other checks would be influenced by this AC_CHECK_HEADERS.
The thing is that there's a following code in readline.h:
#if defined (USE_VARARGS) && defined (PREFER_STDARG) extern int rl_message (const char *, ...) __attribute__((__format__ (printf, 1, 2))); #else extern int rl_message (); #endif
The latter generates the described warning and the former does not.
PREFER_STDARG and USE_VARARGS come from another readline header, rlstdc.h, and it contains the following:
#if defined (__STDC__) && defined (HAVE_STDARG_H) # define PREFER_STDARG # define USE_VARARGS #else # if defined (HAVE_VARARGS_H) # define PREFER_VARARGS # define USE_VARARGS # endif #endif
So we need to have HAVE_STDARG_H defined to have a proper declaration.
How do you like if I rephrase it like "... and add a check for stdarg.h so we have HAVE_STDARG_H defined that's needed by the readline headers to use proper rl_message declaration"?
Yes, that is clear enough even for me :) Jan

Ján Tomko wrote:
What is the purpose of checking for stdarg.h?
As the commit message says: "... and add a check for stdarg.h so readline headers use proper rl_message declaration." I thought that's enough details, but obviously it's not as you're asking :-)
I skipped that part of the commit message. I did not expect that other checks would be influenced by this AC_CHECK_HEADERS.
The thing is that there's a following code in readline.h:
#if defined (USE_VARARGS) && defined (PREFER_STDARG) extern int rl_message (const char *, ...) __attribute__((__format__ (printf, 1, 2))); #else extern int rl_message (); #endif
The latter generates the described warning and the former does not.
PREFER_STDARG and USE_VARARGS come from another readline header, rlstdc.h, and it contains the following:
#if defined (__STDC__) && defined (HAVE_STDARG_H) # define PREFER_STDARG # define USE_VARARGS #else # if defined (HAVE_VARARGS_H) # define PREFER_VARARGS # define USE_VARARGS # endif #endif
So we need to have HAVE_STDARG_H defined to have a proper declaration.
How do you like if I rephrase it like "... and add a check for stdarg.h so we have HAVE_STDARG_H defined that's needed by the readline headers to use proper rl_message declaration"?
Yes, that is clear enough even for me :)
Jan
Pushed with the commit message updated, thanks! Roman Bogorodskiy
participants (2)
-
Ján Tomko
-
Roman Bogorodskiy