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

Roman Bogorodskiy posted 1 patch 6 years, 10 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20170602150423.59568-1-bogorodskiy@gmail.com
configure.ac        | 2 +-
m4/virt-readline.m4 | 4 +++-
tools/Makefile.am   | 3 ++-
3 files changed, 6 insertions(+), 3 deletions(-)
[libvirt] [PATCH v2] virsh: workaround readline prototypes warnings
Posted by Roman Bogorodskiy 6 years, 10 months ago
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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] virsh: workaround readline prototypes warnings
Posted by Ján Tomko 6 years, 10 months ago
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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] virsh: workaround readline prototypes warnings
Posted by Roman Bogorodskiy 6 years, 10 months ago
  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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] virsh: workaround readline prototypes warnings
Posted by Ján Tomko 6 years, 10 months ago
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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] virsh: workaround readline prototypes warnings
Posted by Roman Bogorodskiy 6 years, 10 months ago
  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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list