[PATCH] sim: add --map-info option
Mike Frysinger
vapier@gentoo.org
Tue Dec 14 15:17:00 GMT 2010
On Tuesday, December 14, 2010 02:35:58 Joel Brobecker wrote:
> > + for (memory = STATE_CORE (sd), nr_map = 0; nr_map < nr_maps;
++nr_map)
>
> 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 the
for statement ? initializing things once that dont change inside of the loop.
by this same logic, you could say i should hoist the nr_map assignment out
too. or is the multiple initializing assignments undesirable ?
> > + if (nr_map <= io_map)
> > + sim_io_printf (sd, "%s maps:\n",
> > + (nr_map == read_map) ? "read" :
> > + (nr_map == write_map) ? "write" :
> > + (nr_map == exec_map) ? "exec" :
> > + /*(nr_map == io_map) ?*/ "io");
> > + else
> > + sim_io_printf (sd, "??? (%u) maps:\n", nr_map);
>
> I rather you used a case statement or series of if's, in this case
> than assume that there are only 4 values <= 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:
>
> /* Return "read", or "write", or ... if valid nr_map.
> Otherwise return null; */
>
> char *
> io_map_to_str (unsigned nr_map)
> {
> [...]
>
> And then you can use that function to do:
>
> map_str = 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 != 0)
> > + sim_io_printf (sd, "0x%lx:", (long) mapping->space);
> > + sim_io_printf (sd, "0x%08lx", (long) mapping->base);
> > + if (mapping->level != 0)
> > + sim_io_printf (sd, "@0x%lx", (long) mapping->level);
> > + sim_io_printf (sd, ",0x%lx", (long) mapping->nr_bytes);
> > + modulo = mapping->mask + 1;
> > + if (modulo != 0)
> > + sim_io_printf (sd, "%%0x%lx", (long) modulo);
>
> 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 above
my new block. i guess my new code could review the types and do it right.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://sourceware.org/pipermail/gdb-patches/attachments/20101214/4f6d1374/attachment.sig>
More information about the Gdb-patches
mailing list