Re: [PATCH v2] build: silence a clang warning in virsh.c

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]