From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 32451 invoked by alias); 14 Dec 2010 15:17:11 -0000 Received: (qmail 32434 invoked by uid 22791); 14 Dec 2010 15:17:09 -0000 X-SWARE-Spam-Status: No, hits=-2.0 required=5.0 tests=AWL,BAYES_00,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from smtp.gentoo.org (HELO smtp.gentoo.org) (140.211.166.183) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 14 Dec 2010 15:17:04 +0000 Received: from vapier.localnet (localhost [127.0.0.1]) by smtp.gentoo.org (Postfix) with ESMTP id 59CF31B40B1; Tue, 14 Dec 2010 15:17:02 +0000 (UTC) From: Mike Frysinger To: Joel Brobecker Subject: Re: [PATCH] sim: add --map-info option Date: Tue, 14 Dec 2010 15:17:00 -0000 User-Agent: KMail/1.13.5 (Linux/2.6.37-rc5; KDE/4.5.2; x86_64; ; ) Cc: gdb-patches@sourceware.org, Stephen.Kilbane@analog.com, Stuart.Henderson@analog.com, David.Gibson@analog.com References: <1291219863-18458-1-git-send-email-vapier@gentoo.org> <20101214073558.GS2596@adacore.com> In-Reply-To: <20101214073558.GS2596@adacore.com> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="nextPart1660423.ofZgFoW9AY"; protocol="application/pgp-signature"; micalg=pgp-sha1 Content-Transfer-Encoding: 7bit Message-Id: <201012141016.23030.vapier@gentoo.org> X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2010-12/txt/msg00230.txt.bz2 --nextPart1660423.ofZgFoW9AY Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Content-length: 2392 On Tuesday, December 14, 2010 02:35:58 Joel Brobecker wrote: > > + for (memory =3D STATE_CORE (sd), nr_map =3D 0; nr_map < nr_maps;=20 ++nr_map) >=20 > Wouldn't it make sense to hoist the assignment to memory out of > the for initial assignment? It's never changed during the loop... i dont understand ... isnt this the whole purpose of the first clause of th= e=20 for statement ? initializing things once that dont change inside of the lo= op.=20=20 by this same logic, you could say i should hoist the nr_map assignment out= =20 too. or is the multiple initializing assignments undesirable ? > > + if (nr_map <=3D io_map) > > + sim_io_printf (sd, "%s maps:\n", > > + (nr_map =3D=3D read_map) ? "read" : > > + (nr_map =3D=3D write_map) ? "write" : > > + (nr_map =3D=3D exec_map) ? "exec" : > > + /*(nr_map =3D=3D io_map) ?*/ "io"); > > + else > > + sim_io_printf (sd, "??? (%u) maps:\n", nr_map); >=20 > I rather you used a case statement or series of if's, in this case > than assume that there are only 4 values <=3D io_map and thus if it's > not read_map or write_map or exec_map, then it's io_map. i dont really like that idea > How about: >=20 > /* Return "read", or "write", or ... if valid nr_map. > Otherwise return null; */ >=20 > char * > io_map_to_str (unsigned nr_map) > { > [...] >=20 > And then you can use that function to do: >=20 > map_str =3D io_map_to_str (nr_map); > if (map_str) > sim_io_printf ("%s maps:\n", map_str); > else > sim_io_printf ("??? (%u) maps:\n", nr_map); but this idea is good, so i'll implement it > > + sim_io_printf (sd, " map "); > > + if (mapping->space !=3D 0) > > + sim_io_printf (sd, "0x%lx:", (long) mapping->space); > > + sim_io_printf (sd, "0x%08lx", (long) mapping->base); > > + if (mapping->level !=3D 0) > > + sim_io_printf (sd, "@0x%lx", (long) mapping->level); > > + sim_io_printf (sd, ",0x%lx", (long) mapping->nr_bytes); > > + modulo =3D mapping->mask + 1; > > + if (modulo !=3D 0) > > + sim_io_printf (sd, "%%0x%lx", (long) modulo); >=20 > I don't understand the necessity to cast everything to long. Can you > explain? it's taken largely unchanged from the OPTION_MEMORY_INFO case block just ab= ove=20 my new block. i guess my new code could review the types and do it right. -mike --nextPart1660423.ofZgFoW9AY Content-Type: application/pgp-signature; name=signature.asc Content-Description: This is a digitally signed message part. Content-length: 836 -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.16 (GNU/Linux) iQIcBAABAgAGBQJNB4pGAAoJEEFjO5/oN/WB09MP+gNLKDHAj0UvNXL4pDESM3XP 6q2qjvdArGCsn0ZYei/dZVMsx5ofimJza9CFrllT0p1/997VFAgVRzVzLnhbzg9a HGC3eGI6olYG1wiNyrBDnFlqx+XmA+3HjMph0oXur4vJnqM1F+C/8lldWQl0+EJy 3caViP6iuRoHDt0QR5aN4i6u+JnhDqFP2wVbcBX9H9EllwM1SWrQSxMWVqj7c25t Sn2EoGx0syziq3rGGGB8CYsyTHmVU1UWbM6GVwiMC6UzBqSLb6+Q4BJkzZcydzrP 1vKJmLHurZHG3Tb36alnTHGPDzUH5y6W1kKGOGuQY3LPaq+3+F6D+OS7gVVfZrs4 l8P9MuVn9Kbhf4i/gPTJTGUmKHl6ujG/o+rf8HHlfBSaBv8HfJsitvwO+kLWQ9FM TLIsWJzo54TZAMw++1CnzYj8zkJKCyRySmN/rovDxH++iMFXeioo8L/pNb0DxFiw /uhfL/xRj86u+LY14YecryO5OZal2Pa4BPTCjqOtfuMLKO/VM5duTEbrBeAx6PdT Tr4k+zWacDWHiuUcb1BoMI0orzQx1W7Fu+pdor9PiOh0eg2FGdpx/GikWNfTraqk GVlW5/GbO97LMXjLLQWcY6GeQTX5ECQJ071Ea1FB250YhedtiCUmWfPZ4lULVKEg h5M1BGyU1S4znMeZcy3x =hXs3 -----END PGP SIGNATURE----- --nextPart1660423.ofZgFoW9AY--