[libvirt] [PATCH v2] build: silence a clang warning in virsh.c
This patch shuts up the following warning of clang on Mac OS X: virsh.c:2761:22: error: assigning to 'char *' from 'const char [6]' discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers] rl_readline_name = "virsh"; ^ ~~~~~~~ The warning happens because rl_readline_name on Mac OS X is still 'char *', while it is 'const char *' on most platforms. So adding a cast to (char *) can suppress the warning, although that is not necessary for other platforms. Tested on Mac OS X 10.8.5 (clang-500.2.75) and Fedora 19 (gcc 4.8.1). Signed-off-by: Ryota Ozaki <ozaki.ryota@gmail.com> --- tools/virsh.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 241c5b8..1edffb3 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -2757,8 +2757,12 @@ vshReadlineInit(vshControl *ctl) int max_history = 500; const char *histsize_str; - /* Allow conditional parsing of the ~/.inputrc file. */ - rl_readline_name = "virsh"; + /* Allow conditional parsing of the ~/.inputrc file. + * XXX: the cast is necessary for Mac OS X to slient + * clang's warning. On Mac OS X, rl_readline_name is + * char * while on most platforms it's const char *. + */ + rl_readline_name = (char *) "virsh"; /* Tell the completer that we want a crack first. */ rl_attempted_completion_function = vshReadlineCompletion; -- 1.8.4
On 11/18/2013 08:39 AM, Ryota Ozaki wrote:
This patch shuts up the following warning of clang on Mac OS X:
Not just clang, but also gcc.
virsh.c:2761:22: error: assigning to 'char *' from 'const char [6]' discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers] rl_readline_name = "virsh"; ^ ~~~~~~~
The warning happens because rl_readline_name on Mac OS X is still 'char *', while it is 'const char *' on most platforms.
This is the real bug we are working around, not clang. What version of libreadline is installed on your platform? I'm okay with the patch, but not the commit message - I want to accurately report that WHY we are tweaking things is to work around a bug in older libreadline (which has nothing to do with which compiler is used, and everything to do with the buggy older header). -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 11/18/2013 09:01 AM, Eric Blake wrote:
On 11/18/2013 08:39 AM, Ryota Ozaki wrote:
This patch shuts up the following warning of clang on Mac OS X:
Not just clang, but also gcc.
virsh.c:2761:22: error: assigning to 'char *' from 'const char [6]' discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers] rl_readline_name = "virsh"; ^ ~~~~~~~
The warning happens because rl_readline_name on Mac OS X is still 'char *', while it is 'const char *' on most platforms.
This is the real bug we are working around, not clang.
What version of libreadline is installed on your platform? I'm okay with the patch, but not the commit message - I want to accurately report that WHY we are tweaking things is to work around a bug in older libreadline (which has nothing to do with which compiler is used, and everything to do with the buggy older header).
Actually, the patch itself also needs a tweak:
- /* Allow conditional parsing of the ~/.inputrc file. */ - rl_readline_name = "virsh"; + /* Allow conditional parsing of the ~/.inputrc file. + * XXX: the cast is necessary for Mac OS X to slient
No need for 'XXX:', as there is nothing to fix further once we document why we have a cast. s/slient/silence/
+ * clang's warning. On Mac OS X, rl_readline_name is + * char * while on most platforms it's const char *.
Again, I think it's better to call out the faulty version of libreadline with the problem, rather than which compiler chokes on it. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 11/18/2013 09:03 AM, Eric Blake wrote:
The warning happens because rl_readline_name on Mac OS X is still 'char *', while it is 'const char *' on most platforms.
This is the real bug we are working around, not clang.
Again, I think it's better to call out the faulty version of libreadline with the problem, rather than which compiler chokes on it.
I did some history digging. bash.git is not the world's friendliest git repository (Chet insists on one mega commit per release rather than individual commits per logical change, although he's at least gotten better at weekly patches rather than waiting for major releases) - but in spite of that, it only took me about 10 minutes to prove that rl_readline_name was introduced non-const in bash 2.0.1 (and its corresponding readline version 2.1 in June 1997) and made const in bash 2.0.5 (and its corresponding readline version 4.2, released in Apr 2001). Sheesh, is MacOS really using something older than readline 4.2, just because they are anti-GPLv3? -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Hi Eric, Thank you for very kind replying and investigations. And I'm sorry I'm slow to react. On Tue, Nov 19, 2013 at 1:44 AM, Eric Blake <eblake@redhat.com> wrote:
On 11/18/2013 09:03 AM, Eric Blake wrote:
The warning happens because rl_readline_name on Mac OS X is still 'char *', while it is 'const char *' on most platforms.
This is the real bug we are working around, not clang.
Again, I think it's better to call out the faulty version of libreadline with the problem, rather than which compiler chokes on it.
I did some history digging. bash.git is not the world's friendliest git repository (Chet insists on one mega commit per release rather than individual commits per logical change, although he's at least gotten better at weekly patches rather than waiting for major releases) - but in spite of that, it only took me about 10 minutes to prove that rl_readline_name was introduced non-const in bash 2.0.1 (and its corresponding readline version 2.1 in June 1997) and made const in bash 2.0.5 (and its corresponding readline version 4.2, released in Apr 2001).
I have also dug readline.h. /usr/include/readline/readline.h (actually it's a link to ../editline/readline.h) on Mac OS X says that: /* $NetBSD: readline.h,v 1.30 2009/09/07 21:24:34 christos Exp $ */ /*- * Copyright (c) 1997 The NetBSD Foundation, Inc. * All rights reserved. So as your investigation found out, the header file seems to come from around 1997 (NetBSD 1.2.1?) and not upgraded until now. NetBSD of course has migrated to a recent readline (6.2 according to ftp://ftp.netbsd.org/pub/pkgsrc/current/pkgsrc/devel/readline/README.html).
Sheesh, is MacOS really using something older than readline 4.2, just because they are anti-GPLv3?
Maybe so... Anyway thanks again, ozaki-r
-- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 11/19/2013 02:09 AM, Ryota Ozaki wrote:
Hi Eric,
Thank you for very kind replying and investigations. And I'm sorry I'm slow to react.
On Tue, Nov 19, 2013 at 1:44 AM, Eric Blake <eblake@redhat.com> wrote:
On 11/18/2013 09:03 AM, Eric Blake wrote:
The warning happens because rl_readline_name on Mac OS X is still 'char *', while it is 'const char *' on most platforms.
This is the real bug we are working around, not clang.
Again, I think it's better to call out the faulty version of libreadline with the problem, rather than which compiler chokes on it.
I did some history digging. bash.git is not the world's friendliest git repository (Chet insists on one mega commit per release rather than individual commits per logical change, although he's at least gotten better at weekly patches rather than waiting for major releases) - but in spite of that, it only took me about 10 minutes to prove that rl_readline_name was introduced non-const in bash 2.0.1 (and its corresponding readline version 2.1 in June 1997) and made const in bash 2.0.5 (and its corresponding readline version 4.2, released in Apr 2001).
I have also dug readline.h. /usr/include/readline/readline.h (actually it's a link to ../editline/readline.h) on Mac OS X says that:
/* $NetBSD: readline.h,v 1.30 2009/09/07 21:24:34 christos Exp $ */
/*- * Copyright (c) 1997 The NetBSD Foundation, Inc. * All rights reserved.
So as your investigation found out, the header file seems to come from around 1997 (NetBSD 1.2.1?) and not upgraded until now.
NetBSD of course has migrated to a recent readline (6.2 according to ftp://ftp.netbsd.org/pub/pkgsrc/current/pkgsrc/devel/readline/README.html).
The lastest readline.h from libedit still uses 'extern char *rl_readline_name: /* $NetBSD: readline.h,v 1.34 2013/05/28 00:10:34 christos Exp $ */ http://cvsweb.netbsd.org/bsdweb.cgi/src/lib/libedit/readline/readline.h?rev=1.34&content-type=text/x-cvsweb-markup&sortby=date Jan
On 11/19/2013 02:30 AM, Ján Tomko wrote:
I have also dug readline.h. /usr/include/readline/readline.h (actually it's a link to ../editline/readline.h) on Mac OS X says that:
Ah, so the bug is not in using an ancient readline, but in the fact that editline is not drop-in compatible with readline, contrary to their claims.
The lastest readline.h from libedit still uses 'extern char *rl_readline_name:
Sounds like someone that cares about editline should report an upstream bug. At any rate, the patch that I pushed is still correct, so libvirt should no longer be affected by the problem. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
From: Ryota Ozaki <ozaki.ryota@gmail.com> This patch shuts up the following warning of clang on Mac OS X: virsh.c:2761:22: error: assigning to 'char *' from 'const char [6]' discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers] rl_readline_name = "virsh"; ^ ~~~~~~~ The warning happens because rl_readline_name on Mac OS X comes from an old readline header that still uses 'char *', while it is 'const char *' in readline 4.2 (April 2001) and newer. Tested on Mac OS X 10.8.5 (clang-500.2.75) and Fedora 19 (gcc 4.8.1). Signed-off-by: Ryota Ozaki <ozaki.ryota@gmail.com> Signed-off-by: Eric Blake <eblake@redhat.com> --- Pushing this version. Diff from v2 is solely in the commit message and code comment, and I already expressed ack to the code change of v2. tools/virsh.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 241c5b8..5559d71 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -2757,8 +2757,11 @@ vshReadlineInit(vshControl *ctl) int max_history = 500; const char *histsize_str; - /* Allow conditional parsing of the ~/.inputrc file. */ - rl_readline_name = "virsh"; + /* Allow conditional parsing of the ~/.inputrc file. + * Work around ancient readline 4.1 (hello Mac OS X), + * which declared it as 'char *' instead of 'const char *'. + */ + rl_readline_name = (char *) "virsh"; /* Tell the completer that we want a crack first. */ rl_attempted_completion_function = vshReadlineCompletion; -- 1.8.3.1
participants (3)
-
Eric Blake -
Ján Tomko -
Ryota Ozaki