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

List:       openjdk-awt-dev
Subject:    Re: <AWT Dev> RFR (M) 8223261 "JDK-8189208 followup - remove JDK_GetVersionInfo0 and the supporting 
From:       David Holmes <david.holmes () oracle ! com>
Date:       2019-11-14 22:04:06
Message-ID: 1fc99211-7b79-cc8c-b54f-b7de27294a95 () oracle ! com
[Download RAW message or body]

On 15/11/2019 1:13 am, gerard ziemski wrote:
> Thank you for the review.
> 
> I'm definitively going to wait for Christoph to check in his fix first.
> 
> I tried in fact to leave jdk_util.c/.h files in empty without removing 
> them from the repo, and even though Mac/Linux were OK with that, 
> Solaris/Windows were not.

The .c file can go. The .h file wouldn't be empty as it still has the 
other includes.

David

> 
> cheers
> 
> On 11/13/19 9:21 PM, David Holmes wrote:
>> Hi Gerard,
>>
>> On 14/11/2019 6:05 am, Langer, Christoph wrote:
>>> Hi Gerard,
>>>
>>> generally it looks like a nice cleanup.
>>>
>>> I've got a patch prepared though, which I was planning on posting 
>>> tomorrow. It is about cleanup for the canonicalize function in 
>>> libjava. I wanted to use jdk_util.h for the function prototype. I had 
>>> not yet filed a bug but here is what I have:
>>> http://cr.openjdk.java.net/~clanger/webrevs/cleanup-canonicalize/
>>>
>>> So maybe you could refrain from removing jdk_util.h or maybe you can 
>>> hold off submitting your change until my cleanup is reviewed?
>>
>> I'd also suggest not deleting jdk_util.h. It seems very odd to me to 
>> have jdk_util_md.h with no shared jdk_util.h. If you keep jdk_util.h 
>> then you don't need to touch a number of the files.
>>
>> Otherwise this looks like a good cleanup.
>>
>> Thanks,
>> David
>> -----
>>
>>> I'll create a bug and post an official review thread tomorrow...
>>>
>>> Thanks
>>> Christoph
>>>
>>>> -----Original Message-----
>>>> From: hotspot-dev <hotspot-dev-bounces@openjdk.java.net> On Behalf Of
>>>> Mandy Chung
>>>> Sent: Mittwoch, 13. November 2019 20:30
>>>> To: gerard ziemski <gerard.ziemski@oracle.com>
>>>> Cc: awt-dev@openjdk.java.net; hotspot-dev developers <hotspot-
>>>> dev@openjdk.java.net>; core-libs-dev@openjdk.java.net
>>>> Subject: Re: RFR (M) 8223261 "JDK-8189208 followup - remove
>>>> JDK_GetVersionInfo0 and the supporting code"
>>>>
>>>>
>>>>
>>>> On 11/13/19 10:50 AM, gerard ziemski wrote:
>>>>> Hi all,
>>>>>
>>>>> Please review this cleanup, where we remove JDK_GetVersionInfo0 and
>>>>> related code, since we can get build versions directly from within the
>>>>> VM itself:
>>>>>
>>>>> I'm including core-libs and awt in this review because the proposed
>>>>> fix touches their corresponding files.
>>>>>
>>>>>
>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8223261
>>>>> webrev: http://cr.openjdk.java.net/~gziemski/8223261_rev1
>>>>> tests: passes Mach5 tier1,2,3,4,5,6
>>>>>
>>>>
>>>> This is a good clean up.  JDK_GetVersionInfo0 was needed long time ago
>>>> in particular in HS express support that is no longer applicable.
>>>>
>>>> One leftover comment should also be removed.
>>>>
>>>> src/hotspot/share/runtime/vm_version.hpp
>>>>     // Gets the jvm_version_info.jvm_version defined in jvm.h
>>>>
>>>> otherwise looks good.
>>>>
>>>> Mandy
> 
[prev in list] [next in list] [prev in thread] [next in thread]