[PATCH] Added summary info to GCDAProfiling.

[Justin Bogner]

LGTM

Yuchen Wu <yuchenericwu at hotmail.com> writes:
>>> + if (val != (uint32_t)-1) {
>>> + /* There are counters present in the file. Merge them. */
>>
>> I'm pretty sure this comment makes no sense here - copy pasted from the
>> other function?
>
> In this case, the counters the comment is referring to are the
> programs (which technically never changes) and runs (which does
> change).
>
>>> + val = read_32bit_value(); // length
>>> + if (val == (uint32_t)-1 || val != 9) {
>>
>> This seems a little redundant. Also, maybe this 9 should have a name,
>> since we need it a few times.
>
> Good catch, also introduced a const int now.
>
>>
>>> + fprintf(stderr, "profiling:invalid object length (%d)\n", val); // length
>>> + return;
>>> + }
>>> +
>>> + read_32bit_value(); // checksum, unused
>>> + read_32bit_value(); // num, unused
>>> + runs += read_32bit_value(); // add previous run count to new counter
>>> + }
>>> +
>>> + cur_pos = save_cur_pos;
>>> +
>>> + /* Object summary tag */
>>> + write_bytes("\0\0\0\xa1", 4);
>>> + write_32bit_value(9); // hard-coded to 9 to be gcov-compatible
>>
>> The constant should make this comment unnecessary.
>>
>>> + write_32bit_value(0);
>>> + write_32bit_value(0);
>>
>> It'd be nice to note what these fields are.
>
> They were noted earlier in the read section, but I'll add them here
> too for clarity.
>
>>
>>> + write_32bit_value(runs);
>>> + for (i = 0; i < 6; ++i) write_32bit_value(0);
>>
>> It's clearer what's going on if we say 3..9 here, since we can then use
>> our constant. Also, I think it's more readable with a newline before the
>> loop body.
>
> Done.
>
> I've attached the new patch with your suggestions added.