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

[Wenchao Xia <xiawenc@linux.vnet.ibm.com>]

于 2013-6-28 5:26, Eric Blake 写道:
> On 06/26/2013 09:27 PM, Wenchao Xia wrote:
>> Parameter *mon is added to replace *cur_mon, and readline_completion()
>> pass rs->mon as value, which should be initialized in readline_init()
>> called by monitor_init(). In short, structure ReadLineState controls
>> where the action would be taken now.
>>
>> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>> ---
>
>> @@ -4075,20 +4075,27 @@ static void file_completion(const char *input)
>>               if (stat(file, &sb) == 0 && S_ISDIR(sb.st_mode)) {
>>                   pstrcat(file, sizeof(file), "/");
>>               }
>> -            readline_add_completion(cur_mon->rs, file);
>> +            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? 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.

-- 
Best Regards

Wenchao Xia