Re: [libvirt] [PATCH v1 2/4] remote: Implement remote{Get,Set}Time
On 02/14/2014 06:34 AM, Daniel P. Berrange wrote:
On Fri, Feb 14, 2014 at 06:32:21AM -0700, Eric Blake wrote:
> On 02/14/2014 06:23 AM, Martin Kletzander wrote:
>> On Thu, Feb 13, 2014 at 07:51:43PM +0100, Michal Privoznik wrote:
>>> This is also adding new ACL permission to check 'set_time'.
>>>
>>> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
>>> ---
>
>>
>> Applies to previous patch too, this 'time' will be a problem with
>> '-Wshadow-declarations' on older (some) compilers.
>>
>> ACK with that variable name changed.
>
> I'm half-tempted to just tweak m4/virt-compile-warnings.m4 to drop
> -Wshadow-declarations on older gcc. Since newer gcc is sane about local
> variables not conflicting with public functions, it's not worth worrying
> about the collisions that only older gcc reports.
The problem is shadow decls can occur within libvirt code too in which
case they would likely be genuine bugs. eg someone declares 'foo' at
the start of a method and some time later redeclares it in a for/while
loop body or some such.
Yes, but -Wshadow-declarations catches that on newer gcc. Thus, my
proposal is:
older gcc: omit the warning option, since it is prone to noise that devs
on newer systems have to fix after the fact
newer gcc: use -Wshadow-declarations, and catch the real problems (and
not the conflict between global functions and local variables)
Most dev work is done on newer gcc and thus will avoid the real
problems, and patches submitted from devs on older gcc may cause issues
that have to be fixed up by devs on newer machines, but it will be less
frequent than the case of devs submitting patches that then cause old
gcc to barf on
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 604 bytes)