From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55800) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UsYwH-0005ct-P8 for qemu-devel@nongnu.org; Fri, 28 Jun 2013 09:42:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UsYwF-0005IJ-Ne for qemu-devel@nongnu.org; Fri, 28 Jun 2013 09:42:13 -0400 Received: from mx1.redhat.com ([209.132.183.28]:57947) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UsYwF-0005IA-GX for qemu-devel@nongnu.org; Fri, 28 Jun 2013 09:42:11 -0400 Message-ID: <51CD92AD.3060805@redhat.com> Date: Fri, 28 Jun 2013 07:42:05 -0600 From: Eric Blake MIME-Version: 1.0 References: <1372303666-12323-1-git-send-email-xiawenc@linux.vnet.ibm.com> <1372303666-12323-2-git-send-email-xiawenc@linux.vnet.ibm.com> <51CCADFF.4040805@redhat.com> <51CCF436.1010904@linux.vnet.ibm.com> In-Reply-To: <51CCF436.1010904@linux.vnet.ibm.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="----enig2NVEAAAGGQBPNCSEGQETC" Subject: Re: [Qemu-devel] [PATCH V3 1/7] monitor: discard global variable *cur_mon in completion functions List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Wenchao Xia Cc: pbonzini@redhat.com, lcapitulino@redhat.com, qemu-devel@nongnu.org, armbru@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) ------enig2NVEAAAGGQBPNCSEGQETC Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable 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 an= y >> 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 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 wit= h >> global functions. >> > Thanks for reviewing. I'll fix what you mentioned in v2. >=20 > For the -Wshadow warning, I guess you mean "file" right? No, I meant ffs. ffs() is a global function, in scope if 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. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org ------enig2NVEAAAGGQBPNCSEGQETC Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.13 (GNU/Linux) Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJRzZKtAAoJEKeha0olJ0Nqn9cH/1fN2LzqJMPYrzkfdNkkQ/e5 a29CrOsBvNzrcqETlqtsVGvJ3N8ruKWYMBy+jcjyQp5ggx/Fz8k4+TvccYYo6PK7 SYCLiOxMXJ+55rL9soZQNuBPoJLsV6M6uhUvUC1DO9DYlOLdWuY5vKsSQ++w67Po 5fep/5+8Jv3lc5iryc5gnwmlSg3UHWszOSujR7LDqehO5KQoOjNMsPW+RAEIYROD AGe36BNkPf3/jEywBUDTnEXBrXZ4/I2Cuil8nt8Q8UI2ccHsyYchokM75/t2Nk6l Ib3MkUIaQ/dPJttZRM/4a/4LawjG/7DYXMfJRmKu3a0Cp4UqJAk/9gqBvw1YXCM= =pURQ -----END PGP SIGNATURE----- ------enig2NVEAAAGGQBPNCSEGQETC--