Re: GDB / SIM 7.3.1 Cosmetic patch for profile title

Joel Brobecker
2011-10-19T00:44:27Z

> The issue being that "Summary profiling results" is displayed multiple
> times in a row. This patch fixes profile_info so that the title is
> only displayed once.
[...]
> Mon Oct 17 00:14:40 EDT 2011 John Wehle (john@feith.com)
>
> * sim-profile.c (profile_info): Only print the title once.

This is OK with one little nit:

> - for (c = 0; c < MAX_NR_PROCESSORS; ++c)
> + for (c = 0; c < MAX_NR_PROCESSORS && ! print_title_p; ++c)
^^^^^^^^^^^^^^^
No space after the '!'.

As far as I can tell, you do not have a copyright assignment in
place for GDB (it looks like you assigned some specific changes
many many years ago, but nothing that covers those changes).
I will commit them using the "small change" rule, but if you think
you might contribute other changes, I invite you to start the process
now, so that future patches do not stay held up just because of
paperwork.

As a side note, it took me a while to understand some of the logic.
It looks like `print_title_p' really means `we-have-some-profile-data'.

So, if it was me, I'd probably rename that variable to something
like: profile_data_collected_p, and then rewrite the code such that
the double-loop only sets that variable. And then have a simple
if block:

if (profile_data_collected_p)
profile_printf (sd, cpu, "Summary profiling results:\n\n");

The only other place where this variable is reference would also benefit
from that renaming, since...

if (print_title_p
&& (PROFILE_INSN_P (cpu)
|| PROFILE_MODEL_P (cpu)))
profile_print_addr_ranges (cpu);

... would become:

if (profile_data_collected_p
&& (PROFILE_INSN_P (cpu)
|| PROFILE_MODEL_P (cpu)))
profile_print_addr_ranges (cpu);

--
Joel