RFR 8026324: hs_err improvement: Add summary section to hs_err file

[David Holmes]

On 22/07/2015 4:07 AM, Coleen Phillimore wrote:
>
> I'm going to cut out some comments that have been replied to.
>
> On 7/20/15 4:35 AM, David Holmes wrote:
>> Hi Coleen,
>> On 18/07/2015 5:16 AM, Coleen Phillimore wrote:
>>>
>>> Hi David,  Thank you for taking the time to look and make comments on
>>> all of this.   I have a new webrev at:
>>>
>>> 1713       mhz = 0;
>>>
>>> Wouldn't a default of 1 be safer incase anyone parses this and does
>>> arithmetic?
>>>
>>> Hm, maybe I shouldn't report it if it's not available instead.
>>
>> My only concern with not reporting MHz if the sysctl fails is again
>> for anyone trying to parse the output - they won't know from a good
>> log that a different format may be possible.
>
> Your suggestion of '1' though seems really odd for anyone parsing this
> too.  Why would that be safer than 0?  It seems like 0 would be the
> numerator or multiplier in any calculation.  I like 0 or nothing better
> than 1 in any case.  I'm going back to zero if you think parsers will be
> used.

My concern with 0 is that if used in arithmetic it will propagate and 
that may eventually lead to division by zero somewhere. With 1 that 
can't happen so it is safer.

Cheers,
David

> ...
>>>> 2082     char buf[257];
>>>> j
>>>> Where does the 257 line length come from? Is this  a defined limit for
>>>> the release file?
>>>>
>>>> 2218       char buf[257];
>>>>
>>>> Where does the 257 line length come from? The flags line on my linux
>>>> box is 410 characters.
>>>
>>> I found 257 in another place and thought it seemed good enough for the
>>> cpuinfo line, because it's not looking for the 'flags' line.  I wonder
>>> if there's an advantage to a power of two for a local stack buffer?  I
>>> could go to 512 but I'm trying not to use stack space.
>>
>> Okay but it reads the entire file so some lines will be processed in
>> multiple parts. That doesn't seem to be an issue though.
>
> These use fgets which reads to newlines, so the line would have to be >
> 256 to not make sense (the lines are not longer than 256 where the CPU
> information or os information is stored).  It is longer for 'flags' but
> I'm throwing that line away.
>>
>>>
>>>>
>>>> 2247   ARM32_ONLY("arm32") PPC_ONLY("PPC64") AARCH64_ONLY("AARCH64"),
>>>> length);
>>>>
>>>> May I suggest "ARM" for ARM32 and it should be "AArch64".
>>>
>>> Okay.
>>
>> I meant "ARM" as opposed to "ARM32".
>
> Okay, I changed it to ARM.
>>
>> Also this:
>>
>> 2252   strncpy(cpuinfo, IA32_ONLY("x86 IA32") AMD64_ONLY("x86 IA64")
>>
>> potentially confuses x64 with the obsolete IA64. How about x86_32 and
>> x86_64 ?
>
> Yes, this is a good suggestion.   I fixed it.  I can't remember where I
> got this string from.
>
> ...
>
> Thanks David!
>
> Coleen
>
>> :)
>>
>> Thanks Coleen!
>>
>> David
>> -----
>>
>>>
>>> Coleen
>>>>
>>>> Thanks,
>>>> David
>>>> -----
>>>>
>>>>> Thanks,
>>>>> Coleen
>>>
>