RFR 8026324: hs_err improvement: Add summary section to hs_err file
coleen.phillimore at oracle.com
Tue Jul 21 18:07:48 UTC 2015
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
>> 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
>>> 2082 char buf;
>>> Where does the 257 line length come from? Is this a defined limit for
>>> the release file?
>>> 2218 char buf;
>>> 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"),
>>> May I suggest "ARM" for ARM32 and it should be "AArch64".
> 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 Coleen!
More information about the hotspot-dev