[9] RFR 8146653: Debug version missing in hs_err files and on internal version after Verona

[David Holmes]

On 20/01/2016 3:05 AM, Alejandro E Murillo wrote:
>
> On 1/17/2016 9:05 PM, David Holmes wrote:
>> Hi Alejandro,
>>
>> On 16/01/2016 9:31 AM, Alejandro E Murillo wrote:
>>>
>>> Hi Dan, thanks a lot for the prompt review
>>> I  applied all the changes, revised webrev (with proper links) is here:
>>>
>>> http://cr.openjdk.java.net/~amurillo/9/8146653/index.html
>>
>> Seeing this all together now I don't think alternate_jdk_debug_level
>> warrants being added to Abstract_VM_version. The logic could be used
>> directly inline in VMError.cpp. If it were used more than once then
>> perhaps ... though printable_jdk_debug_level might be a better name in
>> that case (still not a great name but better IMHO :) ).
> Now that you mention it, there's some library code that may also use it.
> There's also the code to check for not provided, which will have to
> replicate,
> so I think I'll keep it. But I'll change the name to "printable"

Ok.

Thanks,
David

> Thanks for this review and the discussion prior to sending this out
> Alejandro
>
>>
>> Thanks,
>> David
>>
>>
>>> Thanks
>>> Alejandro
>>>
>>> On 1/15/2016 3:09 PM, Daniel D. Daugherty wrote:
>>>> On 1/15/16 2:51 PM, Alejandro E Murillo wrote:
>>>>>
>>>>> Please review the following fix for JDK-8146653
>>>>> <https://bugs.openjdk.java.net/browse/JDK-8146653>:
>>>>> Debug version missing in hs_err files and on internal version after
>>>>> Verona
>>>>>
>>>>> Webrev: http://cr.openjdk.java.net/~amurillo/9/8146653/
>>>>
>>>> Your version of webrev generated broken nav links for
>>>> the "frames" version. Shows 404...
>>>>
>>>>
>>>> src/share/vm/runtime/vm_version.cpp
>>>>     L242:   return strcmp(DEBUG_LEVEL, "release") ?
>>>>     L243:       VMNAME " (" DEBUG_LEVEL " " INTERNAL_VERSION_SUFFIX
>>>>     L244:           :
>>>>     L245:       VMNAME " (" INTERNAL_VERSION_SUFFIX;
>>>>
>>>>         No implied booleans on this part: strcmp(DEBUG_LEVEL,
>>>> "release")
>>>>
>>>>         Also I think this would be more HotSpot style:
>>>>
>>>>         return strcmp(DEBUG_LEVEL, "release") == 0
>>>>             ? VMNAME " (" INTERNAL_VERSION_SUFFIX
>>>>             : VMNAME " (" DEBUG_LEVEL " " INTERNAL_VERSION_SUFFIX;
>>>>
>>>>         where the "usual case" is first and the indenting is more
>>>>         HotSpot like...
>>>>
>>>>     L258:   return strcmp(DEBUG_LEVEL, "release") ? DEBUG_LEVEL " "
>>>> : "";
>>>>
>>>>         Similarly here:
>>>>
>>>>         return strcmp(DEBUG_LEVEL, "release") == 0 ? "" : DEBUG_LEVEL;
>>>>
>>>> src/share/vm/runtime/vm_version.hpp
>>>>     No comments.
>>>>
>>>> src/share/vm/utilities/vmError.cpp
>>>>     L234:    const char* runtime_version =
>>>> JDK_Version::runtime_version() != NULL ?
>>>>     L235: JDK_Version::runtime_version() : "";
>>>>     L236:    const char* jdk_debug_level =
>>>> Abstract_VM_Version::alternate_jdk_debug_level() != NULL ?
>>>>     L237: Abstract_VM_Version::alternate_jdk_debug_level() : "";
>>>>
>>>>         L237 indent needs three more spaces. So does L235, but that's
>>>>         not your issue.
>>>>
>>>>     L239:    st->print_cr("# JRE version: %s (%s) (%sbuild %s)",
>>>> runtime_name, buf,
>>>>     L240:                  jdk_debug_level, runtime_version);
>>>>
>>>>         L240 indent has one too many spaces.
>>>>
>>>> Dan
>>>>
>>>>>
>>>>> Background:
>>>>> Verona removed the debug info from version related system properties
>>>>> and that's now specified on a new system property called jdk.debug.
>>>>> The version header spitted in hs_err files was updated to include
>>>>> debug info based on that change.
>>>>> That info was also missing on the output for -Xinternalversion
>>>>>
>>>>> Thanks
>>>>>
>>>>
>>>
>