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

[Wenchao Xia]

于 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 <xiaw...@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