This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: GDB / SIM 7.3.1 Cosmetic patch for profile title


> 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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]