Re: [Qemu-devel] [PATCH V3 1/7] monitor: discard global variable *cur_mon in completion functions

[Eric Blake]

On 06/27/2013 08:25 PM, Wenchao Xia wrote:
>>> +            readline_add_completion(mon->rs, file);
>>>           }
>>>       }
>>>       closedir(ffs);
>>
>> Oops, I started reviewing that before noticing you posted a v3.  At any
>> rate, my review of v2 still stands on this patch.  Meanwhile, I just
>> barely noticed that this pre-existing code might cause a -Wshadow
>> warning in the future if <strings.h> is ever included.  I'm not sure
>> what qemu's policy is on being clean against -Wshadow warnings, but I
>> personally tend to avoid local variables whose name might conflict with
>> global functions.
>>
>   Thanks for reviewing. I'll fix what you mentioned in v2.
> 
>   For the -Wshadow warning, I guess you mean "file" right?

No, I meant ffs.  ffs() is a global function, in scope if <strings.h> is
included; so using it as a local variable name will trigger -Wshadow
warnings from (at least some versions of) gcc.

> It would
> be a bit hard to check this warning for new patch, since this warning
> now exist in many code. In my opinion, it would be better to have a
> separate series to clean up this warnings first, then check new patches
> for this issue.

Agreed that the issue (of using ffs as a local variable name) is
pre-existing, and therefore cleanups for -Wshadow are not related to
this series and not something you need to necessarily worry about.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature