[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-hotspot-dev
Subject:    Re: RFR : 8217786: Provide virtualization related info in the hs_error file on linux s390x
From:       David Holmes <david.holmes () oracle ! com>
Date:       2019-01-30 22:47:51
Message-ID: 8ab96b97-ba38-76ff-9f28-27f29ca19567 () oracle ! com
[Download RAW message or body]

Matthias,

Turned out this broke the build on non S390 as the compiler complained 
about the unused function print_matching_lines_from_sysinfo_file. In the 
rush to fix that 8218060 incorrectly removed the function completely. 
But then 8218063 put it back in an ifdef. So all should be well again.

Please ensure all changes go through jdk-submit before pushing.

Thanks,
David

On 30/01/2019 7:00 pm, Baesken, Matthias wrote:
> Hi David,
> 
>>> Style nit: avoid implicit booleans, explicitly check != NULL
> 
> I  added the explicit  "!= NULL" check and  an add a line  with an  introductory text   .
> 
> 
> @Thomas  -  may I add you as reviewer  ?
> 
> 
> Thanks, Matthias
> 
> 
>> -----Original Message-----
>> From: David Holmes <david.holmes@oracle.com>
>> Sent: Mittwoch, 30. Januar 2019 06:49
>> To: Baesken, Matthias <matthias.baesken@sap.com>; Thomas Stüfe
>> <thomas.stuefe@gmail.com>
>> Cc: hotspot-dev@openjdk.java.net
>> Subject: Re: RFR : 8217786: Provide virtualization related info in the hs_error
>> file on linux s390x
>>
>> Hi Matthias,
>>
>> Thanks for reworking this.
>>
>> On 30/01/2019 2:56 am, Baesken, Matthias wrote:
>>> Hello, I added a break to avoid potential printing lines multiple times,
>>> and removed the comment line :
>>>
>>> http://cr.openjdk.java.net/~mbaesken/webrevs/8217786.3/
>>
>> A couple of minor comments:
>>
>> src/hotspot/os/linux/os_linux.cpp
>>
>> +     while (keywords_to_match[i]) {
>>
>> Style nit: avoid implicit booleans, explicitly check != NULL
>>
>> + void os::Linux::print_virtualization_info(outputStream* st) {
>>
>> Don't you want an initial print of some introductory text eg:
>>
>> "Virtualization Information"
>>
>> No need for updated webrev.
>>
>> Thanks,
>> David
>> -----
> 
[prev in list] [next in list] [prev in thread] [next in thread]